[m-rev.] for post-commit review: explain grades more deeply

Julien Fischer jfischer at opturion.com
Mon May 4 15:25:33 AEST 2020


Hi Zoltan,

Mon, 4 May 2020, Zoltan Somogyi wrote:

> 2020-05-04 13:43 GMT+10:00 Julien Fischer<jfischer at opturion.com>:
>> Do you want me to commit these changes?
>
> I thought you already had.

Nope.

> Some parts I agree with, some parts I don't.
>
>>> I agree, the description of the grade system should be in its own
>>> section.  Given the level of confusion that they seem to cause, maybe
>>> it should be its own chapter instead?
>
> For a chapter, I think we would need a complete description
> of which grade modifiers are compatible with which base grades
> and *other* modifiers. For that, I would prefer to wait until
> we switch to using the grade library, since its structured grade
> representation is I think the only way we can *enforce* whatever
> rules the documentation says.

That's fine.

>>> diff --git a/doc/user_guide.texi b/doc/user_guide.texi
>>> index 288fabd..ea19b4a 100644
>>> --- a/doc/user_guide.texi
>>> +++ b/doc/user_guide.texi

...

>>> -The available base grades are the following.
>>> +The available base grades are:
>
> I think your wording, which is also the old wording, works *only*
> if what follows are part of the same sentence. I added "the following"
> because each point is now a complete sentence.

Ok, I've changed it back.

>>> @ item @samp{hlc}
>>> This base grade calls for generating idiomatic C,
>>> -which we call high level C.
>>> +which we call high-level C.
>
> As is clear by my use of high level C, I prefer not to use the dashed version.

The non dashed version is not consistent with:

    - the majority of the rest of the documentation for the Mercury system.
    - the compiler's usage message
    - the project's website (sometimes)
    - the title of the paper describing the "high-level C" backend.

I have no objection to the non dashed forms of "low level" and "high level"
but such a change should be made everywhere, not just in one spot.

(While we're at it, we should probably decide on one of: "back-end",
"back end" or "backend".)

...

>>> +By default when targeting C, the Mercury @samp{float} type is implemented as > an
>>> +IEEE 754 double-precision floating point number, which occupies 64 bits of
>>> +space.
>
> Two things wrong here. First it restricts the following discussion to C, but then
> says nothing about the other target languages.

I've appended the following to it:

     For target languages other than C, the Mercury @samp{float} type is
     always implemented as a double-precision floating point number and the
     @samp{.spf} modifier is not supported.

> Second, we have no control over whether Mercury floats are IEEE 754 or
> not. They will be, on most kinds of platforms, but there are still
> machines out there with non-754 float architectures.

Do we support Mercury on them?  From a quick search, the main candidates
seem to be some IBM mainframes, but they all seem to have FPUs that
support the IEEE binary format as well anyway.

Floats being IEEE 754 are exactly what reference manual currently says
they are.  (And what the runtime and standard library assume.)

>>> @@  -8076,37 +8072,37 @@ small memory segments chained together in a list.
>>>  When there is no room in the current segment for a new stack frame,
>>>  we simply allocate a new segment and add it to the list.
>>>  This approach has higher overhead,
>>> -since calls to and returns from procedures we must execute more code,
>>> +since calls to, and returns from, procedures must execute more code,
>
> In this case, I would prefer a third wording, such as
>
> we must execute more code both at calls to procedures,
> and at returns from them.

Changed to read:

     This approach has higher overhead, since we must execute more code at
     both calls to procedures, and at returns from them.

...

>>> -thread support can be enabled by specifying the grade modifier @samp{.par}.
>>> -When targeting low level C,
>>> +thread support must be enabled by specifying the grade modifier @samp{.par}.
>>> +When targeting low-level C,
>
> Your wording implies that non-par grades are not allowed, which is wrong.

Reverted.

Julien.


More information about the reviews mailing list