You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Patrick Reilly <pa...@gmail.com> on 2014/08/29 19:15:29 UTC
Review Request 25191: Switch [stout] to using compiler intrinsics for
unreachable, exit, and abort
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25191/
-----------------------------------------------------------
Review request for mesos and Benjamin Hindman.
Bugs: MESOS-1744
https://issues.apache.org/jira/browse/MESOS-1744
Repository: mesos-git
Description
-------
Use compiler intrinsics for unreachable, exit, and abort
Makes the functions not need to pretend to return something while
still silencing the compiler warnings.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp f20feea
3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp aaccbb4
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5bbf829
3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 5607896
3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp 3568886
Diff: https://reviews.apache.org/r/25191/diff/
Testing
-------
This is currently a work in progress, (WIP)
Thanks,
Patrick Reilly
Re: Review Request 25191: Switch [stout] to using compiler intrinsics
for unreachable, exit, and abort
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25191/#review54600
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp
<https://reviews.apache.org/r/25191/#comment94787>
make check fails here for me (on clang) because this is not marked inline.
same for the other methods in this diff.
- Dominic Hamon
On Sept. 25, 2014, 12:34 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25191/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2014, 12:34 p.m.)
>
>
> Review request for mesos, Ben Mahler and Dominic Hamon.
>
>
> Bugs: MESOS-1744
> https://issues.apache.org/jira/browse/MESOS-1744
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use compiler intrinsics for unreachable, exit, and abort
> Makes the functions not need to pretend to return something while
> still silencing the compiler warnings.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp f20feea
> 3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp aaccbb4
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5bbf829
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 5607896
> 3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp 3568886
>
> Diff: https://reviews.apache.org/r/25191/diff/
>
>
> Testing
> -------
>
> Make check runs.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25191: Switch [stout] to using compiler intrinsics
for unreachable, exit, and abort
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25191/#review54593
-----------------------------------------------------------
Ship it!
The whole chain looks good to me, can you reach out to Dominic to get the chain committed for you?
3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp
<https://reviews.apache.org/r/25191/#comment94774>
Once you commit this, no one will know about the multi message version you're referring to ;)
I would avoid the TODO altogether and we'll extend _Abort when the need arises.
- Ben Mahler
On Sept. 25, 2014, 7:34 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25191/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2014, 7:34 p.m.)
>
>
> Review request for mesos, Ben Mahler and Dominic Hamon.
>
>
> Bugs: MESOS-1744
> https://issues.apache.org/jira/browse/MESOS-1744
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use compiler intrinsics for unreachable, exit, and abort
> Makes the functions not need to pretend to return something while
> still silencing the compiler warnings.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp f20feea
> 3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp aaccbb4
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5bbf829
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 5607896
> 3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp 3568886
>
> Diff: https://reviews.apache.org/r/25191/diff/
>
>
> Testing
> -------
>
> Make check runs.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25191: Switch [stout] to using compiler intrinsics
for unreachable, exit, and abort
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25191/
-----------------------------------------------------------
(Updated Sept. 25, 2014, 12:34 p.m.)
Review request for mesos, Ben Mahler and Dominic Hamon.
Changes
-------
s/benjaminhindman/bmahler
Ben, could you take a look for sanity check?
Bugs: MESOS-1744
https://issues.apache.org/jira/browse/MESOS-1744
Repository: mesos-git
Description
-------
Use compiler intrinsics for unreachable, exit, and abort
Makes the functions not need to pretend to return something while
still silencing the compiler warnings.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp f20feea
3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp aaccbb4
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5bbf829
3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 5607896
3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp 3568886
Diff: https://reviews.apache.org/r/25191/diff/
Testing
-------
Make check runs.
Thanks,
Patrick Reilly
Re: Review Request 25191: Switch [stout] to using compiler intrinsics
for unreachable, exit, and abort
Posted by Dominic Hamon <dh...@twopensource.com>.
> On Sept. 5, 2014, 2 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp, line 21
> > <https://reviews.apache.org/r/25191/diff/1/?file=672334#file672334line21>
> >
> > can we use __builtin_unreachable instead?
> >
> > we should probably check for it (__has_builtin(__builtin_unreachable)).
>
> Cody Maloney wrote:
> The compiler intrinsic just acts as a hint to the compiler allowing it to suppress some erroneous warnings as well as better optimize the code, so it doesn't really do the same thing. I think the message is useful for us for debugging / dev / recovering from error. In the case of `__builtin_unreachable` the behavior just becomes undefined / the compiler is free to make the code do absolutely anything it wants (Including if the point is reached execute arbitrary other code from the executable).
>
> Could you commit this group of three patches (25191, 25192, 25192)?
I'm waiting for benjaminhindman to take a look.
- Dominic
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25191/#review52506
-----------------------------------------------------------
On Aug. 29, 2014, 10:53 a.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25191/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2014, 10:53 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Dominic Hamon.
>
>
> Bugs: MESOS-1744
> https://issues.apache.org/jira/browse/MESOS-1744
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use compiler intrinsics for unreachable, exit, and abort
> Makes the functions not need to pretend to return something while
> still silencing the compiler warnings.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp f20feea
> 3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp aaccbb4
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5bbf829
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 5607896
> 3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp 3568886
>
> Diff: https://reviews.apache.org/r/25191/diff/
>
>
> Testing
> -------
>
> Make check runs.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25191: Switch [stout] to using compiler intrinsics
for unreachable, exit, and abort
Posted by Cody Maloney <co...@mesosphere.io>.
> On Sept. 5, 2014, 9 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp, line 21
> > <https://reviews.apache.org/r/25191/diff/1/?file=672334#file672334line21>
> >
> > can we use __builtin_unreachable instead?
> >
> > we should probably check for it (__has_builtin(__builtin_unreachable)).
>
> Cody Maloney wrote:
> The compiler intrinsic just acts as a hint to the compiler allowing it to suppress some erroneous warnings as well as better optimize the code, so it doesn't really do the same thing. I think the message is useful for us for debugging / dev / recovering from error. In the case of `__builtin_unreachable` the behavior just becomes undefined / the compiler is free to make the code do absolutely anything it wants (Including if the point is reached execute arbitrary other code from the executable).
>
> Could you commit this group of three patches (25191, 25192, 25192)?
>
> Dominic Hamon wrote:
> I'm waiting for benjaminhindman to take a look.
>
> Cody Maloney wrote:
> ping
This has been sitting here quite a while (I tried pining Ben H directly ~ a week ago). Could you commit them for me? Or would you like me to ping another Mesos commiter to take a quick look?
- Cody
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25191/#review52506
-----------------------------------------------------------
On Aug. 29, 2014, 5:53 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25191/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2014, 5:53 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Dominic Hamon.
>
>
> Bugs: MESOS-1744
> https://issues.apache.org/jira/browse/MESOS-1744
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use compiler intrinsics for unreachable, exit, and abort
> Makes the functions not need to pretend to return something while
> still silencing the compiler warnings.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp f20feea
> 3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp aaccbb4
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5bbf829
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 5607896
> 3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp 3568886
>
> Diff: https://reviews.apache.org/r/25191/diff/
>
>
> Testing
> -------
>
> Make check runs.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25191: Switch [stout] to using compiler intrinsics
for unreachable, exit, and abort
Posted by Cody Maloney <co...@mesosphere.io>.
> On Sept. 5, 2014, 9 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp, line 21
> > <https://reviews.apache.org/r/25191/diff/1/?file=672334#file672334line21>
> >
> > can we use __builtin_unreachable instead?
> >
> > we should probably check for it (__has_builtin(__builtin_unreachable)).
>
> Cody Maloney wrote:
> The compiler intrinsic just acts as a hint to the compiler allowing it to suppress some erroneous warnings as well as better optimize the code, so it doesn't really do the same thing. I think the message is useful for us for debugging / dev / recovering from error. In the case of `__builtin_unreachable` the behavior just becomes undefined / the compiler is free to make the code do absolutely anything it wants (Including if the point is reached execute arbitrary other code from the executable).
>
> Could you commit this group of three patches (25191, 25192, 25192)?
>
> Dominic Hamon wrote:
> I'm waiting for benjaminhindman to take a look.
ping
- Cody
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25191/#review52506
-----------------------------------------------------------
On Aug. 29, 2014, 5:53 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25191/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2014, 5:53 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Dominic Hamon.
>
>
> Bugs: MESOS-1744
> https://issues.apache.org/jira/browse/MESOS-1744
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use compiler intrinsics for unreachable, exit, and abort
> Makes the functions not need to pretend to return something while
> still silencing the compiler warnings.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp f20feea
> 3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp aaccbb4
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5bbf829
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 5607896
> 3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp 3568886
>
> Diff: https://reviews.apache.org/r/25191/diff/
>
>
> Testing
> -------
>
> Make check runs.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25191: Switch [stout] to using compiler intrinsics
for unreachable, exit, and abort
Posted by Cody Maloney <cm...@orlyatomics.com>.
> On Sept. 5, 2014, 9 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp, line 21
> > <https://reviews.apache.org/r/25191/diff/1/?file=672334#file672334line21>
> >
> > can we use __builtin_unreachable instead?
> >
> > we should probably check for it (__has_builtin(__builtin_unreachable)).
The compiler intrinsic just acts as a hint to the compiler allowing it to suppress some erroneous warnings as well as better optimize the code, so it doesn't really do the same thing. I think the message is useful for us for debugging / dev / recovering from error. In the case of `__builtin_unreachable` the behavior just becomes undefined / the compiler is free to make the code do absolutely anything it wants (Including if the point is reached execute arbitrary other code from the executable).
Could you commit this group of three patches (25191, 25192, 25192)?
- Cody
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25191/#review52506
-----------------------------------------------------------
On Aug. 29, 2014, 5:53 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25191/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2014, 5:53 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Dominic Hamon.
>
>
> Bugs: MESOS-1744
> https://issues.apache.org/jira/browse/MESOS-1744
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use compiler intrinsics for unreachable, exit, and abort
> Makes the functions not need to pretend to return something while
> still silencing the compiler warnings.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp f20feea
> 3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp aaccbb4
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5bbf829
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 5607896
> 3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp 3568886
>
> Diff: https://reviews.apache.org/r/25191/diff/
>
>
> Testing
> -------
>
> Make check runs.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25191: Switch [stout] to using compiler intrinsics
for unreachable, exit, and abort
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25191/#review52506
-----------------------------------------------------------
Ship it!
Ship It!
3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp
<https://reviews.apache.org/r/25191/#comment91310>
can we use __builtin_unreachable instead?
we should probably check for it (__has_builtin(__builtin_unreachable)).
- Dominic Hamon
On Aug. 29, 2014, 10:53 a.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25191/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2014, 10:53 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Dominic Hamon.
>
>
> Bugs: MESOS-1744
> https://issues.apache.org/jira/browse/MESOS-1744
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use compiler intrinsics for unreachable, exit, and abort
> Makes the functions not need to pretend to return something while
> still silencing the compiler warnings.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp f20feea
> 3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp aaccbb4
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5bbf829
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 5607896
> 3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp 3568886
>
> Diff: https://reviews.apache.org/r/25191/diff/
>
>
> Testing
> -------
>
> Make check runs.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25191: Switch [stout] to using compiler intrinsics
for unreachable, exit, and abort
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25191/
-----------------------------------------------------------
(Updated Aug. 29, 2014, 5:53 p.m.)
Review request for mesos, Benjamin Hindman and Dominic Hamon.
Changes
-------
Adding Dominic Hamon as reviewer.
Bugs: MESOS-1744
https://issues.apache.org/jira/browse/MESOS-1744
Repository: mesos-git
Description
-------
Use compiler intrinsics for unreachable, exit, and abort
Makes the functions not need to pretend to return something while
still silencing the compiler warnings.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp f20feea
3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp aaccbb4
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5bbf829
3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 5607896
3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp 3568886
Diff: https://reviews.apache.org/r/25191/diff/
Testing
-------
Make check runs.
Thanks,
Patrick Reilly
Re: Review Request 25191: Switch [stout] to using compiler intrinsics
for unreachable, exit, and abort
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25191/
-----------------------------------------------------------
(Updated Aug. 29, 2014, 5:51 p.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
Make check runs.
Bugs: MESOS-1744
https://issues.apache.org/jira/browse/MESOS-1744
Repository: mesos-git
Description
-------
Use compiler intrinsics for unreachable, exit, and abort
Makes the functions not need to pretend to return something while
still silencing the compiler warnings.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp f20feea
3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp aaccbb4
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5bbf829
3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 5607896
3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp 3568886
Diff: https://reviews.apache.org/r/25191/diff/
Testing (updated)
-------
Make check runs.
Thanks,
Patrick Reilly