[m-dev.] Re: pragma fact_table

[Fergus Henderson]

David Matthew OVERTON, you wrote:
> 
> Fergus Henderson wrote:
> > 
> > The C code should be in a
> > `:- pragma c_code("....")' block in library/float.m.
> 
> Here are the changes to add the new `float__hash' predicate.  I thought
> it would be easier to put the C function `hash_float' into the runtime
> (as is done with `hash_string') because it is then available whenever
> it is needed by the fact table lookup code rather than having to
> import the float module.  Is that a good idea or not? 

Short answer: yes.

Long answer:
	Uh, it does have one disadvantage -- specifically, hash_float()
	will get linked into everything, increasing the size of
	statically linked executables that don't use it, whereas if it
	is in the library, the smart linking enabled by the
	`--split-c-files' option can avoid that.

	You wouldn't have to "import the float module" to use
	`hash_float()', all you would need to do is to have a
	declaration for it (either in the generated code that invokes
	it, or in a header file in the runtime), and to link against
	the library (which happens automatically anyway).

	However, despite the above, putting hash_float() in the runtime
	is cleaner, so stick with that approach.

> Should I add a version to float.nu.nl as well?

Yes.

> Tom has suggested an alternative (much more efficient) method that
> uses a union of Float and Word to convert the float's bit pattern to
> an integer (the same as float_to_word() for UNBOXED_FLOATS).  However
> I haven't used this method at the moment because (AFAIK) it can't be
> implemented in prolog.

I think Tom's suggestion is a good one.  I don't think that it is vital
that the Prolog hash__float be the same as the Mercury one (and
given floating point inaccuracies, etc., you couldn't guarantee that
anyway).

Of course if sizeof(Float) > sizeof(Word) then the hash value will be better
if it takes into account all the bits in the float, not just the fird
Word's worth.

Also don't forget to hande the case where sizeof(Float) < sizeof(Word)
properly too.

> runtime/misc.c
> 	Add a new function `hash_float' for hashing floats.

I think it would be better to create a new file mercury_float.c
and put it in that.

(I know that is not consistent with what is done with hash_string(),
but hash_string should be in mercury_string.c not misc.c.)

> runtime/mercury_float.m
> 	Add an `extern' declaration for `hash_float'.

s/.m/.h/

>From our C coding standard: "don't bother qualifying function prototypes
with the `extern' keyword.".

>  %---------------------------------------------------------------------------%
> +% Create a non-negative integer hash value for a float.
> +
> +:- pred float__hash(float, int).
> +:- mode float__hash(in, out) is det.

Delete "non-negative".
If the user wants a non-negative hash value, they can use int__abs.


So, can you please implement Tom's suggestion, address the issues raised
above, and then send another diff?

Thanks,	
	Fergus.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>   |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>   |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3         |     -- the last words of T. S. Garp.