pragma fact_table

[Fergus Henderson]

David Matthew OVERTON, you wrote:
> 
> > > +		compile_facts(PredName, Arity, PredInfo1, FactArgInfos,
> > > +		    ProcStreams, MaybeOutput, 0, NumFacts),
> > > +		io__seen,
> > > +		(
> > > +		    { MaybeOutput = yes(_) },
> > > +			% closing brace for the final fact table
> > > +		    io__write_string(OutputStream, "};\n\n"),
> > > +		    write_fact_table_pointer_array(NumFacts, StructName, 
> > > +			OutputStream, C_HeaderCode2)
> > 
> > This code should be near the code the writes the opening brace.
> > 
> This is about as close as it can get because the closing brace can't be
> written until `compile_facts' has finished.

In that case, the opening brace should be written before `compile_facts'
has started.  Change write_fact_data so that it writes out only matching
pairs of braces.

Oh, and since you'll then be writing out the opening brace in two places,
just as you're currently writing out the close brace in two places,
use some common subroutines to avoid code duplication.

> > > +	Word index;	  /* index into fact table data array 		     */
> > > +		          /* or pointer to hash table for next argument      */
> > > +	short type;	  /* 0 if entry empty, 1 if entry is a pointer to the*/
> > > +			  /* data table, 2 if entry is a pointer to another  */
> > > +			  /* hash table 				     */
> > 
> > It would be more efficient to use a single tagged pointer rather than
> > two fields here.
> > 
> 
> How would the tags work considering index has to be able to represent an
> integer or a pointer?

Use 30-bit integers (or 61-bit, on a 64-bit architecture).

You'll need to use the mkbody() macro to shift the integer value
a few bits left to make room for the tag, and when you want to get
the integer value back, use the unmkbody() macro, which does the
corresponding shift right.

> > > +write_fact_args([], _) --> [].
> > > +write_fact_args([Arg | Args], OutputStream) -->
> > > +	(
> > > +		{ Arg = term__string(String) },
> > > +		io__set_output_stream(OutputStream, OldStream),
> > > +		io__write_string(""""),
> > > +		term_io__quote_string(String),
> > > +		io__write_string(""", "),
> > > +		io__set_output_stream(OldStream, _)
> > 
> > That's buggy: `term_io__quote_string' uses Mercury quoting
> > conventions, not C quoting conventions.
> 
> This probably has no relevance to what I'm doing, but I though I'd
> point it out anyway:
> `term_io__quote_string' seems to use the C convention for octal
> escapes.  The Mercury refrence manual says that an octal escape should
> be terminated by a backslash, but `term_io__quote_string' doesn't seem
> to put it there.  Is this a bug in the code or in the manual?

It is a bug in the code.  It is however deliberate --
it's a hack for backwards compatibility with non-ISO-conforming
Prolog systems that use C-style octal escapes rather than ISO-Prolog-style
octal escapes. 

I'll add some documentation.

> Also, the lexer seems to know what to do whether the terminating
> backslash is there or not.

Yes, again that's for compatibility with non-ISO-conforming Prolog systems.

See the following coment in the code:

			/****** 
				% We don't report this as an error since
				% we need bug-for-bug compatibility with
				% NU-Prolog
			{ Token = error("unterminated octal escape") }
			******/

Unfortunately that doesn't help with the cases that are legal
in both escape styles, but have different semantics.
For example, "\003\004" means `<control-C><control-D>' in C-style,
but it means `<control-C>004' in ISO-prolog-style.

So the bug in term_io.m does have some negative consequences.
I guess we should fix it at some point.

> Anyway, I agree that it shouldn't be used for C strings since that's
> not what its correct purpose is.


> > > +	% Build a lower level hash table.  The main difference to
> > > +	% build_hash_table (above) is that ``sort file lines'' are read from 
> > > +	% a list rather than from the actual sort file.
> > 
> > Couldn't you avoid the code duplication by factoring out the common
> > code into a subroutine?
> > 
> This has already been done with `do_build_hash_table'.

Fair enough.

> > > +	% Hash computation predicate.
> > > +	% Note:  if you change this predicate, you will also need to change
> > > +	% the C code that is output to compute the hash value at runtime.
> > > +	% This C code is generated in `generate_hash_code'.
> > [...]
> > > +		float__abs(Float, Abs),
> > > +		( Abs = 0.0 -> Abs2 = 1e-15 ; Abs2 = Abs ),
> > > +		math__ln(Abs2, Log),
> > > +		math__ceiling(Log, TruncLog),
> > > +		math__exp(TruncLog, Pow),
> > > +		Float2 is Abs2 / Pow * 1e15,
> > > +		float__truncate_to_int(Float2, N1),
> > > +		float__abs(TruncLog, Log2),
> > > +		float__truncate_to_int(Log2, N2),
> > 
> > Due to the usual vagiaries of floating point arithmetic,
> > it strikes me as very dangerous to assume that the C code and the
> > Mercury code will compute the same hash value.
> > For example, what if you are cross-compiling, and the source
> > and target having different floating point representations?
> 
> Yes, I realise the problem here, but I'm not sure what to do about it.
> Any ideas?  

I don't have any good ideas off the top of my head.
But you should at very least clearly document the problem,
and test it very carefully.

It might be worth posting a query to comp.compilers.

> > 
> > > +	% Write out the size of the fact table.
> > > +	{ pred_info_name(PredInfo, Name) },
> > > +	io__write_strings([
> > > +		"const Integer mercury__",
> > > +		Name,
> > > +		"_fact_table_num_facts = "]),
> > > +	io__write_int(NumFacts),
> > > +	io__write_string(";\n\n"),
> > 
> > You need to quote the predicate name.
> 
> I'm not sure what you mean here.  Do you mean use something like
> `term_io__quote_atom' ?

I mean use `llds_out__name_mangle'.
Perhaps I should have said "mangle" rather than "quote".

> > Your code should not need to know the argument passing conventions.
> > You should use the stuff in arg_info.m to avoid any dependencies on
> > `simple' or `compact' argument-passing conventions.
> 
> This was just a quick hack to get things working with the current
> limitations on nondet pragma C.  None of this code will be necessary
> once nondet pragma C is implemented properly  because other parts of
> the compiler will do the mapping of arguments to registers.
> 
> In light of this, is it worth changing this code now?

Yes.  If it's worth committing, then it needs to be done properly.

Otherwise we'll run into the scenario where no-one ever gets around
to implementing nondet pragma C properly, hence your code stays
as it is, and then when someone wants to make changes to the calling
convention they will have problems.

-- 
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.