PNaCl documentation: add undefined behavior

PNaCl documentation: add undefined behavior

The text was sent to the LLVM mailing list, I edited it a bit and restified it. See:
http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-June/063010.html

R= binji@chromium.org, sbc@chromium.org, dschuff@chromium.org, mseaborn@chromium.org, jvoung@chromium.org, stichnot@chromium.org, sehr@chromium.org, ncbray@chromium.org, mackinlay@chromium.org
TEST= none
BUG= none
NOTRY=true
(documentation only change)

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=252826

[binji, 2014-02-19 23:23:34]

https://codereview.chromium.org/164413005/diff/1/native_client_sdk/src/doc/re...
File native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst (right):

https://codereview.chromium.org/164413005/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:70: Well
Defined
I think this is hyphenated

https://codereview.chromium.org/164413005/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:140: * The
incomplete

https://codereview.chromium.org/164413005/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:148: * Zero or
negative-sized variable-length array (and ``alloca``): can
the part after the colon doesn't match the rest of the examples above.

[JF, 2014-02-20 02:24:59]

https://codereview.chromium.org/164413005/diff/1/native_client_sdk/src/doc/re...
File native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst (right):

https://codereview.chromium.org/164413005/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:70: Well
Defined
On 2014/02/19 23:23:34, binji wrote:
> I think this is hyphenated

Done.

https://codereview.chromium.org/164413005/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:140: * The
On 2014/02/19 23:23:34, binji wrote:
> incomplete

Removed, it got left behind in a shuffle.

https://codereview.chromium.org/164413005/diff/1/native_client_sdk/src/doc/re...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:148: * Zero or
negative-sized variable-length array (and ``alloca``): can
On 2014/02/19 23:23:34, binji wrote:
> the part after the colon doesn't match the rest of the examples above.

Done.

[Jim Stichnoth, 2014-02-20 14:15:22]

Mostly grammar nits.

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
File native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst (right):

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:52: through
Chrome updates. This goal is unfortunately not attainable, PNaCl
; instead of ,
... not attainable; PNaCl therefore ...

(Sorry for my pedantry; I have several more of these below, which you can feel
free to ignore.)

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:86: metadata is
stripped from the *pexe*, behavior is therefore
; instead of ,

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:95: same
compatible intrinsics or external functions, the behavior is
; instead of ,

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:118: * There is
still some architecture specific behavior.
architecture-specific

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:136: * Could
force everything to `align 1`, performance cost should be
; instead of ,

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:148: defined
behavior. PNaCl could insert checks with
PNaCl doesn't use -fsanitize.  Either
  PNaCl could insert checks similar to ...
or
  Pnacl-clang could insert checks ...

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:169: * ``NaN``
values are currently not guaranteed to be canonical, see `bug
; instead of ,

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:179:
already-existing *pexe*, it would be an opt-in feature.
; instead of ,

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:181: *
Fused-multiply-add have higher precision and often execute faster,
; instead of ,
or:
... faster, though PNaCl currently...

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:183: currently
doesn't) only generate them in the backend if fast-math
if fast-math were specified
(past subjunctive case :) )

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:185: *
Transcendentals aren't exposed by PNaCl's ABI, they are part of the
; instead of ,

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:215: *
Just-In-Time code generation is supported by NaCl, but will not be
... but is not supported in PNaCl's ...

(given that PNaCl's first release has already happened)

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:217: code as
executable in the first release.
It is currently not possible to mark code as executable.

[JF, 2014-02-20 19:30:49]

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
File native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst (right):

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:52: through
Chrome updates. This goal is unfortunately not attainable, PNaCl
On 2014/02/20 14:15:22, stichnot wrote:
> ; instead of ,
> ... not attainable; PNaCl therefore ...
> 
> (Sorry for my pedantry; I have several more of these below, which you can feel
> free to ignore.)

Done.

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:86: metadata is
stripped from the *pexe*, behavior is therefore
On 2014/02/20 14:15:22, stichnot wrote:
> ; instead of ,

Done.

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:95: same
compatible intrinsics or external functions, the behavior is
On 2014/02/20 14:15:22, stichnot wrote:
> ; instead of ,

Done.

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:118: * There is
still some architecture specific behavior.
On 2014/02/20 14:15:22, stichnot wrote:
> architecture-specific

Done.

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:136: * Could
force everything to `align 1`, performance cost should be
On 2014/02/20 14:15:22, stichnot wrote:
> ; instead of ,

Done.

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:148: defined
behavior. PNaCl could insert checks with
On 2014/02/20 14:15:22, stichnot wrote:
> PNaCl doesn't use -fsanitize.  Either
>   PNaCl could insert checks similar to ...
> or
>   Pnacl-clang could insert checks ...

Done.

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:169: * ``NaN``
values are currently not guaranteed to be canonical, see `bug
On 2014/02/20 14:15:22, stichnot wrote:
> ; instead of ,

Done.

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:179:
already-existing *pexe*, it would be an opt-in feature.
On 2014/02/20 14:15:22, stichnot wrote:
> ; instead of ,

Done.

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:181: *
Fused-multiply-add have higher precision and often execute faster,
On 2014/02/20 14:15:22, stichnot wrote:
> ; instead of ,
> or:
> ... faster, though PNaCl currently...

Done.

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:183: currently
doesn't) only generate them in the backend if fast-math
On 2014/02/20 14:15:22, stichnot wrote:
> if fast-math were specified
> (past subjunctive case :) )

Done.

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:185: *
Transcendentals aren't exposed by PNaCl's ABI, they are part of the
On 2014/02/20 14:15:22, stichnot wrote:
> ; instead of ,

Done.

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:215: *
Just-In-Time code generation is supported by NaCl, but will not be
On 2014/02/20 14:15:22, stichnot wrote:
> ... but is not supported in PNaCl's ...
> 
> (given that PNaCl's first release has already happened)

Done.

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:217: code as
executable in the first release.
On 2014/02/20 14:15:22, stichnot wrote:
> It is currently not possible to mark code as executable.

Done.

[jvoung (off chromium), 2014-02-20 20:41:14]

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
File native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst (right):

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:83: reduces to
``ret i32 undef`` in bitcode.
Should we have set this to return something defined (like 0) and not "undef", in
the ABI stabilization passes? (or trap)

(this doesn't sound so "fixed" in the pexe unless our backend treats undef as
something fixed)

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:160: *
Different rounding modes are currently not usable, which isn't
This doesn't seem like undefined behavior. It's more "missing feature"?

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:171: * It is
currently unspecified whether signaling ``NaN`` faults.
I thought we agreed not to fault? At least the default is no faulting, and we
haven't supplied any way to turn on faulting.

[JF, 2014-02-21 22:49:18]

Note: I created a new section in C/C++ language support for FP, since some of
the UD isn't actually UD.

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
File native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst (right):

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:83: reduces to
``ret i32 undef`` in bitcode.
On 2014/02/20 20:41:14, jvoung (cr) wrote:
> Should we have set this to return something defined (like 0) and not "undef",
in
> the ABI stabilization passes? (or trap)
> 
> (this doesn't sound so "fixed" in the pexe unless our backend treats undef as
> something fixed)

Good point. I'll move this to could fix, and will point to the bug I just filed:
  https://code.google.com/p/nativeclient/issues/detail?id=3796

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:160: *
Different rounding modes are currently not usable, which isn't
On 2014/02/20 20:41:14, jvoung (cr) wrote:
> This doesn't seem like undefined behavior. It's more "missing feature"?

Do we specify which mode is the default? And what happens when the user tries
changing it? I think we're just missing the header, right?

Maybe we just need to document this better. In the C/C++ language support? I'll
move it to an FP section.

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:171: * It is
currently unspecified whether signaling ``NaN`` faults.
On 2014/02/20 20:41:14, jvoung (cr) wrote:
> I thought we agreed not to fault? At least the default is no faulting, and we
> haven't supplied any way to turn on faulting.

True, added to C/C++ language support FP section as discussed above.

[jvoung (off chromium), 2014-02-21 22:59:46]

updates for my comments LGTM

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
File native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst (right):

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:160: *
Different rounding modes are currently not usable, which isn't
On 2014/02/21 22:49:19, JF wrote:
> On 2014/02/20 20:41:14, jvoung (cr) wrote:
> > This doesn't seem like undefined behavior. It's more "missing feature"?
> 
> Do we specify which mode is the default? And what happens when the user tries
> changing it? I think we're just missing the header, right?
> 
> Maybe we just need to document this better. In the C/C++ language support?
I'll
> move it to an FP section.

We make the default round to nearest. At least there is a test for that, but
looks like I forgot to add a not in the docs about that.

There's no way for the user to change it, since there's not an intrinsic (yet)
and we don't allow the assembly that the intrinsic would have done, that they
would need for changing it. So right now the round to nearest is the rounding
mode.

https://codereview.chromium.org/164413005/diff/210001/native_client_sdk/src/d...
File native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst (right):

https://codereview.chromium.org/164413005/diff/210001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:167: * Float
divide by zero is currently undefined.
That's a bit unfortunate since frem div by zero is pretty defined. Anyway, just
an observation =)

[JF, 2014-02-21 23:13:24]

I believe all issues are now addressed. Comment now or forever hold you peace
(by this I mean: or send a subsequent CL to fix issues you didn't comment on
now).

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
File native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst (right):

https://codereview.chromium.org/164413005/diff/50001/native_client_sdk/src/do...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:160: *
Different rounding modes are currently not usable, which isn't
> We make the default round to nearest. At least there is a test for that, but
> looks like I forgot to add a not in the docs about that.
> 
> There's no way for the user to change it, since there's not an intrinsic (yet)
> and we don't allow the assembly that the intrinsic would have done, that they
> would need for changing it. So right now the round to nearest is the rounding
> mode.

Done.

https://codereview.chromium.org/164413005/diff/210001/native_client_sdk/src/d...
File native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst (right):

https://codereview.chromium.org/164413005/diff/210001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/pnacl-undefined-behavior.rst:167: * Float
divide by zero is currently undefined.
On 2014/02/21 22:59:47, jvoung (cr) wrote:
> That's a bit unfortunate since frem div by zero is pretty defined. Anyway,
just
> an observation =)

Yeah this is more about whether FPU control bits are set, how the user checks
them, and whether a HW exception is thrown.

[JF, 2014-02-22 22:04:28]

The CQ bit was checked by jfb@chromium.org

[commit-bot: I haz the power, 2014-02-22 22:04:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jfb@chromium.org/164413005/350001

[commit-bot: I haz the power, 2014-02-22 22:28:20]

Change committed as 252826