meow_hash icon indicating copy to clipboard operation
meow_hash copied to clipboard

MeowU64From only returns the first 64 bytes of the hash

Open Clark-E opened this issue 4 years ago • 4 comments

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

Clark-E avatar Jun 19 '20 00:06 Clark-E

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: https://github.com/cmuratori/meow_hash/blob/5962cb76a1b6d49319ae9cad1ea0e2bfe70443d3/meow_hash_x64_aesni.h#L143

mmozeiko avatar Jun 19 '20 00:06 mmozeiko

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 avatar Jun 19 '20 00:06 Clark-E

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

Clark-E avatar Jun 19 '20 00:06 Clark-E

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

cmuratori avatar Jun 19 '20 04:06 cmuratori