Reference Manual Clarifications

Here is a patch containing all but one of the changes. I realized that the remainder/modulo discussion does indeed belongs to the srem instruction. The semantics of urem are obvious and need no further clarification.

Best Regards,
Jon

1572,1573c1572,1575
< notation (see below). Floating point constants must have a <a
< href="#t_floating">floating point</a> type. </dd>

Hi Jon,

Please you'll want to submit patches as unified diffs and as attachments.

I notice you're using Thunderbird, so I refer you to this tip:

http://lists.cs.uiuc.edu/pipermail/llvmdev/2008-January/011992.html

Although this note doesn't apply to how you included your original patch (looks like you pasted it in), Thunderbird has default attachment handling settings which cause problems for many of our reviewers.

Thanks,
Gordon

Gordon Henriksen wrote:

Hi Jon,

Please you'll want to submit patches as unified diffs and as attachments.

I notice you're using Thunderbird, so I refer you to this tip:

http://lists.cs.uiuc.edu/pipermail/llvmdev/2008-January/011992.html

Although this note doesn't apply to how you included your original patch (looks like you pasted it in), Thunderbird has default attachment handling settings which cause problems for many of our reviewers.

Thanks,
Gordon

Ok, did I get it right this time?

LangRef.html.patch (5.1 KB)

The fptrunc instruction states "If the value cannot fit within the destination type, ty2, then the results are undefined." This is fine, but what about other floating-point operations that can overflow? For example, does 'mul double 1.0e300, 1.0e300' produce +infinity or is it undefined? I think LLVM should treat floating-point overflows consistently. On a similar note, what is the result of division by zero for fdiv? infinity or undefined?

The manual needs to state the rounding mode of the fptrunc, uitofp, and sitofp instruction (if only to say it's undefined). I recommend round-to-nearest.

What is the behavior of alloca if there is insufficient memory? null or undefined behavior?

I'm attaching another round of changes. Please verify that they are correct.

Best Regards,
Jon

langref_changes2.patch (5.06 KB)

What is the behavior of alloca if there is insufficient memory? null or
undefined behavior?

Undefined.

Ciao,

Duncan.

The manual needs to state the rounding mode of the fptrunc, uitofp, and
sitofp instruction (if only to say it's undefined). I recommend
round-to-nearest.

Alternatively, the manual needs to admit the notion of rounding mode
controls per IEEE floating point, and then define the instructions to
operate according to IEEE.

What is the behavior of alloca if there is insufficient memory? null or
undefined behavior?

While you are touching alloca docs, I had a discussion on IRC about it
that should be added: alloca allocates storage from the stack, and is
guaranteed not to trigger GC.

This matters because on some systems the stack frames might be allocated
from the heap. The alloca primitive assumes that this is not the case.

The fptrunc instruction states "If the value cannot fit within the destination type, ty2, then the results are undefined." This is fine, but what about other floating-point operations that can overflow? For example, does 'mul double 1.0e300, 1.0e300' produce +infinity or is it undefined?

It is defined by IEEE to be inf.

I think LLVM should treat floating-point overflows consistently. On a similar note, what is the result of division by zero for fdiv? infinity or undefined?

IEEE also specifies these, but I don't know the answer off-hand.

The manual needs to state the rounding mode of the fptrunc, uitofp, and sitofp instruction (if only to say it's undefined). I recommend round-to-nearest.

Everything is implicitly assumed to be in the default rounding mode.

-Chris

Looks ok, I applied it here:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080331/060531.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080331/060532.html

I removed the "negative or equal" aspect to the shift amount discussion. Shift amounts are interpreted as unsigned values, so they can't be negative.

Also, it looks like your copy of langref is slightly out of date, because the patch didn't apply cleanly.

-Chris

Chris Lattner wrote:

The fptrunc instruction states "If the value cannot fit within the destination type, ty2, then the results are undefined." This is fine, but what about other floating-point operations that can overflow? For example, does 'mul double 1.0e300, 1.0e300' produce +infinity or is it undefined?

It is defined by IEEE to be inf.

Yes. AFAIK, IEEE754 defines floating-point truncation also. If a double-precision value is too large for single precision, the result
is infinity (not undefined).

The manual needs to state the rounding mode of the fptrunc, uitofp, and sitofp instruction (if only to say it's undefined). I recommend round-to-nearest.

Everything is implicitly assumed to be in the default rounding mode.

After doing a quick search on Google, it looks like the default rounding mode is round to nearest. Would the default rounding mode ever be different than round to nearest?

Best Regards,
Jon

Chris Lattner wrote:

Gordon Henriksen wrote:

Hi Jon,
Please you'll want to submit patches as unified diffs and as attachments.
I notice you're using Thunderbird, so I refer you to this tip:
http://lists.cs.uiuc.edu/pipermail/llvmdev/2008-January/011992.html
Although this note doesn't apply to how you included your original patch (looks like you pasted it in), Thunderbird has default attachment handling settings which cause problems for many of our reviewers.
Thanks,
Gordon

Ok, did I get it right this time?

Looks ok, I applied it here:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080331/060531.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080331/060532.html

I removed the "negative or equal" aspect to the shift amount discussion. Shift amounts are interpreted as unsigned values, so they can't be negative.

Ah, I didn't see that in the documentation. I suggest adding "Shift amounts are interpreted as unsigned values" to each shift instruction.

Also, it looks like your copy of langref is slightly out of date, because the patch didn't apply cleanly.

Hmm, I copied LangRef.html from http://llvm.org/docs/LangRef.html. The version I'm using was last modified 2008-03-24 15:52:42 -0500 (Mon, 24 Mar 2008). Perhaps there was a newer version on SVN? In any case, I'll use the latest version from SVN for future patches.

Best Regards,
Jon

I removed the "negative or equal" aspect to the shift amount
discussion. Shift amounts are interpreted as unsigned values, so they
can't be negative.

Ah, I didn't see that in the documentation. I suggest adding "Shift
amounts are interpreted as unsigned values" to each shift instruction.

That would make sense.

Also, it looks like your copy of langref is slightly out of date,
because the patch didn't apply cleanly.

Hmm, I copied LangRef.html from LLVM Language Reference Manual — LLVM 16.0.0git documentation. The
version I'm using was last modified 2008-03-24 15:52:42 -0500 (Mon, 24
Mar 2008). Perhaps there was a newer version on SVN? In any case, I'll
use the latest version from SVN for future patches.

I'm not really sure what is going on, but when I applied the patch, a couple hunks didn't go in. *shrug*, in this case it was easy to fix up by hand.

The fptrunc instruction states "If the value cannot fit within the
destination type, ty2, then the results are undefined." This is fine,

but

what about other floating-point operations that can overflow? For

example,

does 'mul double 1.0e300, 1.0e300' produce +infinity or is it

undefined?

It is defined by IEEE to be inf.

Yes. AFAIK, IEEE754 defines floating-point truncation also. If a
double-precision value is too large for single precision, the result
is infinity (not undefined).

I would have to look this up, but the meta point stands: we should be very clear in langref what the actual semantics are :).

Incidentally, at some point I would like to generalize our FP support to better capture rounding modes and other information (to [e.g.] support the C99 numerics pragmas, -ffast-math, etc). This is a long term project that isn't even very well formed at this point. In the short term, correctly describing what we have is very important and useful. Thank you for working on this!

The manual needs to state the rounding mode of the fptrunc, uitofp,

and

sitofp instruction (if only to say it's undefined). I recommend
round-to-nearest.

Everything is implicitly assumed to be in the default rounding mode.

After doing a quick search on Google, it looks like the default rounding
mode is round to nearest. Would the default rounding mode ever be
different than round to nearest?

Nope, I think that stating the LLVM IR depends on that is fine. Thanks again Jon,

-Chris

Applied with edits:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080331/060556.html

I figured out what your patches don't apply. Something (your web browser, editor, etc) is stripping trailing whitespace.

-Chris

Chris Lattner wrote:

I'm attaching another round of changes. Please verify that they are correct.

Applied with edits:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080331/060556.html

I figured out what your patches don't apply. Something (your web browser, editor, etc) is stripping trailing whitespace.

-Chris

Hmm, I realized that the alignment doesn't have a type in front of it (unlike NumElements) so it's reasonably clear that it's a constant. Saying "constant alignment" isn't necessary.

Regarding malloc, I think your wording isn't clear enough: "Allocating zero bytes is undefined." My understanding is that an undefined operation is illegal; however, the implementation is not required to diagnose it. Allocating zero bytes is legal; it's just that the result is meaningless. Consider rewording as "Allocating zero bytes is legal, but the result is undefined. The result of a zero-sized allocation is a valid argument for free."

Regarding free, I also think your wording isn't clear enough: "If the pointer is null, the result is undefined." The free result is void.
Consider rewording as "If the pointer is null, the operation is valid but does not free the pointer."

Regarding alloca, please add "The operation is undefined if there is insufficient memory available."

Regarding malloc and alloca, I realized that the size is unsigned, so a negative value for NumElements is impossible. I suggest replacing "it is the number of elements allocated" with "it is the UNSIGNED number of elements allocated".

Regarding shl, ashr, and lshr, please add "The second argument is interpreted as an unsigned value."

Regarding unwind, replace "The 'unwind' intrinsic" with "The 'unwind' instruction"

I'm working on another set of changes now. If it's not too much trouble, it would be more convenient for me to post the changes (as I've done in this e-mail) and let you integrate them into LangRef.html.

Best Regards,
Jon

:frowning:

I'd be happy to submit a patch to fix llvm so that it doesn't contain any, thus, reducing the chance that this happens to people. :slight_smile: Let me know if you're interested.

Not worth the noise, IMO.

-Chris

Jon Sargeant wrote:

Chris Lattner wrote:

I'm attaching another round of changes. Please verify that they are correct.

Applied with edits:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080331/060556.html

I figured out what your patches don't apply. Something (your web browser, editor, etc) is stripping trailing whitespace.

-Chris

Hmm, I realized that the alignment doesn't have a type in front of it (unlike NumElements) so it's reasonably clear that it's a constant. Saying "constant alignment" isn't necessary.

Regarding malloc, I think your wording isn't clear enough: "Allocating zero bytes is undefined." My understanding is that an undefined operation is illegal; however, the implementation is not required to diagnose it. Allocating zero bytes is legal; it's just that the result is meaningless. Consider rewording as "Allocating zero bytes is legal, but the result is undefined. The result of a zero-sized allocation is a valid argument for free."

Regarding free, I also think your wording isn't clear enough: "If the pointer is null, the result is undefined." The free result is void.
Consider rewording as "If the pointer is null, the operation is valid but does not free the pointer."

Regarding alloca, please add "The operation is undefined if there is insufficient memory available."

Regarding malloc and alloca, I realized that the size is unsigned, so a negative value for NumElements is impossible. I suggest replacing "it is the number of elements allocated" with "it is the UNSIGNED number of elements allocated".

Regarding shl, ashr, and lshr, please add "The second argument is interpreted as an unsigned value."

Regarding unwind, replace "The 'unwind' intrinsic" with "The 'unwind' instruction"

I'm working on another set of changes now. If it's not too much trouble, it would be more convenient for me to post the changes (as I've done in this e-mail) and let you integrate them into LangRef.html.

Best Regards,
Jon

Chris, I'm awaiting your reply.

Best Regards,
Jon

Jon Sargeant wrote:

Jon Sargeant wrote:

Chris Lattner wrote:

I'm attaching another round of changes. Please verify that they are correct.

Applied with edits:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080331/060556.html

I figured out what your patches don't apply. Something (your web browser, editor, etc) is stripping trailing whitespace.

-Chris

Hmm, I realized that the alignment doesn't have a type in front of it (unlike NumElements) so it's reasonably clear that it's a constant. Saying "constant alignment" isn't necessary.

Regarding malloc, I think your wording isn't clear enough: "Allocating zero bytes is undefined." My understanding is that an undefined operation is illegal; however, the implementation is not required to diagnose it. Allocating zero bytes is legal; it's just that the result is meaningless. Consider rewording as "Allocating zero bytes is legal, but the result is undefined. The result of a zero-sized allocation is a valid argument for free."

Regarding free, I also think your wording isn't clear enough: "If the pointer is null, the result is undefined." The free result is void.
Consider rewording as "If the pointer is null, the operation is valid but does not free the pointer."

Regarding alloca, please add "The operation is undefined if there is insufficient memory available."

Regarding malloc and alloca, I realized that the size is unsigned, so a negative value for NumElements is impossible. I suggest replacing "it is the number of elements allocated" with "it is the UNSIGNED number of elements allocated".

Regarding shl, ashr, and lshr, please add "The second argument is interpreted as an unsigned value."

Regarding unwind, replace "The 'unwind' intrinsic" with "The 'unwind' instruction"

I'm working on another set of changes now. If it's not too much trouble, it would be more convenient for me to post the changes (as I've done in this e-mail) and let you integrate them into LangRef.html.

Best Regards,
Jon

Chris, I'm awaiting your reply.

Best Regards,
Jon
_______________________________________________
LLVM Developers mailing list
LLVMdev@cs.uiuc.edu http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

Still waiting.

Chris Lattner wrote:

I'm attaching another round of changes. Please verify that they are correct.

Applied with edits:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080331/060556.html

Hmm, I realized that the alignment doesn't have a type in front of it
(unlike NumElements) so it's reasonably clear that it's a constant.
Saying "constant alignment" isn't necessary.

It doesn't hurt.

Regarding malloc, I think your wording isn't clear enough: "Allocating
zero bytes is undefined." My understanding is that an undefined
operation is illegal; however, the implementation is not required to
diagnose it. Allocating zero bytes is legal; it's just that the result
is meaningless. Consider rewording as "Allocating zero bytes is legal,
but the result is undefined. The result of a zero-sized allocation is a
valid argument for free."

Sure, that makes sense.

Regarding free, I also think your wording isn't clear enough: "If the
pointer is null, the result is undefined." The free result is void.
Consider rewording as "If the pointer is null, the operation is valid
but does not free the pointer."

It isn't though. free(NULL) could segfault the app, for example.

Regarding alloca, please add "The operation is undefined if there is
insufficient memory available."

Makes sense.

Regarding malloc and alloca, I realized that the size is unsigned, so a
negative value for NumElements is impossible. I suggest replacing "it
is the number of elements allocated" with "it is the UNSIGNED number of
elements allocated".

I'm not sure why this is more clear.

Regarding shl, ashr, and lshr, please add "The second argument is
interpreted as an unsigned value."

Seems reasonable. This is redundant with other wording, but is more clear.

Regarding unwind, replace "The 'unwind' intrinsic" with "The 'unwind'
instruction"

Done.

I'm working on another set of changes now. If it's not too much
trouble, it would be more convenient for me to post the changes (as I've
done in this e-mail) and let you integrate them into LangRef.html.

It would definitely be easier to integrate in patch form. Here is what I committed:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080414/061277.html

-Chris

Chris Lattner wrote:

Thanks for your reply.

Chris Lattner wrote:

Regarding free, I also think your wording isn't clear enough: "If the
pointer is null, the result is undefined." The free result is void.
Consider rewording as "If the pointer is null, the operation is valid
but does not free the pointer."

It isn't though. free(NULL) could segfault the app, for example.

See Nick's post.

Regarding malloc and alloca, I realized that the size is unsigned, so a
negative value for NumElements is impossible. I suggest replacing "it
is the number of elements allocated" with "it is the UNSIGNED number of
elements allocated".

I'm not sure why this is more clear.

The semantics of malloc and alloca depend on whether you interpret NumElements as a signed or unsigned 32-bit integer. For example, if NumElements is 0xffffFFFF, should the instruction fail (because allocating a negative number of elements doesn't make sense), or should the instruction allocate 2^32-1 elements? I don't see any mention of whether NumElements is signed or unsigned in the documentation.

I'm working on another set of changes now. If it's not too much
trouble, it would be more convenient for me to post the changes (as I've
done in this e-mail) and let you integrate them into LangRef.html.

It would definitely be easier to integrate in patch form. Here is what I committed:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080414/061277.html

Very well. I'll submit future changes as patches.

Best Regards,
Jon