[m-rev.] for review: ROTOR fixes

[Fergus Henderson]

On 05-Mar-2003, Peter Ross <pro at missioncriticalit.com> wrote:
> Get the mercury compiler working with ROTOR.  The MS shared source CLI
> implementation.

s/.  The/, the/

> compiler/mlds_to_il.m:
> 	ROTOR doesn't allow calli calls to be made tail calls, so
> 	don't make them tail calls.

That should be conditional on a compiler option, called say
"--support-rotor-clr".  Furthermore, IMHO it should not be enabled by
default.  "tail.calli" is needed for tail calling in nondeterministic
Mercury code.  We shouldn't turn that off to work around bugs in Rotor;
not optimizing tail calls is itself a bug, because it can lead to memory
leaks.  IMHO Rotor support is not important enough to be worth damaging
things for non-Rotor CLRs.  Rotor is IMHO the least important of the
four CLR implementations.

> runtime/mercury_il.il:
> 	Remove the tail. prefix to the calli instructions.

Similar issue here.  Not quite so important, since it only relates
to higher-order tail calls, but still worth getting right for non-ROTOR
implementations IMHO.

In this case, we can't use a compiler option.
But you could mark the lines in question with a special comment,

	tail.		// NOT FOR ROTOR
	calli ...

and use "grep -v" to delete them, or "sed" to comment them out.
This could be done in part of the Mmakefile which is not enabled
by default, e.g. like this:

   # Uncomment the following rule if you want to build Mercury
   # in a form that will work with ROTOR.
   #mercury_il.dll:
   #	sed '/NOT FOR ROTOR/s@^@//@' mercury_il.il > mercury_il.rotor.il
   #	$(MS_ILASM) $(ALL_MS_ILASMFLAGS) /dll /quiet /OUT=mercury_il.dll \
   #		mercury_il.rotor.il

(Better test this, since I haven't :-)

> library/array.m:
> 	Fix some function signatures which were incorrectly declared
> 	as returning int rather than MR_bool, which the MS CLR accepts
> 	but ROTOR doesn't.

Fine, thanks.  Actually I had the same diff in my tree too, since
Portable.NET doesn't accept those either.  I just hadn't gotten
around to committing it.

> +++ compiler/mlds_to_il.m	5 Mar 2003 10:30:59 -0000
> @@ -1619,7 +1619,10 @@
>  		\+ (
>  			{ MsCLR = yes },
>  			{ ReturnParam \= CallerReturnParam }
> -		)
> +		),
> +		% The ROTOR implementation only allows const calls to be tail
> +		% calls.
> +		{ Function = const(_) }
>  	->
>  		{ TailCallInstrs = [tailcall] },
>  		% For calls marked with "tail.", we need a `ret'

I found the comment here confusing, since the ROTOR implementation doesn't
have any concept of a "const call".  Mentioning "const" here is just
duplicating the code; it doesn't help explain why the code is there.

So I suggest wording the comment as follows:

		% The ROTOR implementation only allows "tail."
		% annotations on direct calls (tail.call),
		% not indirect calls (calli).

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------