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

MeowU64From only returns the first 64 bytes of the hash #74

Open
Clark-E opened this issue Jun 19, 2020 · 4 comments
Open

MeowU64From only returns the first 64 bytes of the hash #74

Clark-E opened this issue Jun 19, 2020 · 4 comments

Comments

@Clark-E
Copy link

Clark-E commented Jun 19, 2020

When trying to get the result of a computed hash using MeowU64From, only the first 64 bytes are returned, regardless of the value of the second function argument.

Here's a simple c/c++ program I wrote to demonstrate the issue:

#include <iostream>
#include "meowHash.h" //the header file that contains the Meow Hash source code

void main(){
	
	//fill buffer with data.
	
	int bufferSize = 50;
	
	char* buffer = (char*)malloc(bufferSize);
	
	for(int i = 0; i < bufferSize; i++){
		
		buffer[i] = i;
		
	}
	
	//hash and print.
	
	meow_u128 hash = MeowHash(MeowDefaultSeed,bufferSize,buffer);
	
	long long unsigned hash1 = MeowU64From(hash,0);
	long long unsigned hash2 = MeowU64From(hash,1);
	
	std::cout << std::hex << hash1 << " " << hash2 << std::dec << std::endl;
	
	long unsigned hash3 = MeowU32From(hash,0);
	long unsigned hash4 = MeowU32From(hash,1);
	long unsigned hash5 = MeowU32From(hash,2);
	long unsigned hash6 = MeowU32From(hash,3);
	
	std::cout << std::hex << hash3 << " " << hash4 << " " << hash5 << " " << hash6 << std::dec << std::endl;
	
}

When run, it produces this output:

f977fc4881456a98 f977fc4881456a98
81456a98 f977fc48 d3d8ef6a 4e7a8b47

The U32 parts can be accessed fine, but trying to access the hash as a pair of U64 parts does not work, as it instead only returns the first 64 bytes.

Compiler: Microsoft Visual Studio Community 2019 16.4.5 using Visual C++ 2019
CPU: Intel Core i5-9300H CPU
OS: 64 bit Windows 10

@mmozeiko
Copy link
Contributor

Are you compiling your code as 32-bit code?

It seems there is a bug for 32-bit case - somebody forgot to put [I] on this line:

#define MeowU64From(A, I) (*(meow_u64 *)&(A))

@Clark-E
Copy link
Author

Clark-E commented Jun 19, 2020

I compiled via "x64_x86", which I think is 32 bit? I'll try compiling and running it natively, but my compiler seems to be having some trouble right now.

@Clark-E
Copy link
Author

Clark-E commented Jun 19, 2020

I compiled and ran it for x64. It works fine when compiled as such. Thank you!

@cmuratori
Copy link
Owner

So actually this problem may be a bit more troublesome, actually. We probably shouldn't have supported 32-bit mode, but I think Jeff wanted it so we did. Nobody is really testing it though :/

Anyway, the problem is that many SIMD instruction sets we want to support cannot do vector extract with a GPR index. So providing a function that takes an index to extract is bad, since in the 32-bit mode, we just compile that as an array access. That would lead someone to think that they could pass a variable here, whereas actually, for any extraction instruction, it has to be a constant. And then when they went to compile in 64-bit mode, they would suddenly get an error.

So probably what needs to happen here is we need to move to something like:

#define MeowU64From0(...)
#define MeowU64From1(...)

#define MeowU32From0(...)
#define MeowU32From1(...)
#define MeowU32From2(...)
#define MeowU32From3(...)

The only other alternative I can think of is simply not supporting it, and if you want to extract values, you have to do it yourself.

- Casey

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants