Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add option to control whether pyimath uses the fp exception mechanism #538

Merged
merged 2 commits into from
Aug 29, 2019
Merged

Add option to control whether pyimath uses the fp exception mechanism #538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cffec0b
Add option to control whether pyimath uses the fp exception mechanism
kdt3rd Aug 24, 2019
cca2fd3
Add missing include
kdt3rd Aug 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion PyIlmBase/PyImath/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,6 @@ pyilmbase_define_module(imath
PyImathVec4Impl.h
PyImathVecOperators.h
DEPENDENCIES
IlmBase::Iex IlmBase::IexMath IlmBase::Imath
IlmBase::Iex IlmBase::IexMath IlmBase::Imath PyIlmBase::Config
MODULE_DEPS PyIex
)
20 changes: 10 additions & 10 deletions PyIlmBase/PyImath/PyImathAutovectorize.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ struct VectorizedFunction1 {
result_type retval = create_uninitalized_return_value<result_type>::apply(len);
VectorizedOperation1<Op,result_type,arg1_type> vop(retval,arg1);
dispatchTask(vop,len);
mathexcon.handleOutstandingExceptions();
PY_IMATH_RETURN_PYTHON;
return retval;
}

Expand Down Expand Up @@ -474,7 +474,7 @@ struct VectorizedFunction2 {
result_type retval = create_uninitalized_return_value<result_type>::apply(len);
VectorizedOperation2<Op,result_type,arg1_type,arg2_type> vop(retval,arg1,arg2);
dispatchTask(vop,len);
mathexcon.handleOutstandingExceptions();
PY_IMATH_RETURN_PYTHON;
return retval;
}

Expand Down Expand Up @@ -507,7 +507,7 @@ struct VectorizedFunction3 {
result_type retval = create_uninitalized_return_value<result_type>::apply(len);
VectorizedOperation3<Op,result_type,arg1_type,arg2_type,arg3_type> vop(retval,arg1,arg2,arg3);
dispatchTask(vop,len);
mathexcon.handleOutstandingExceptions();
PY_IMATH_RETURN_PYTHON;
return retval;
}

Expand Down Expand Up @@ -682,7 +682,7 @@ struct VectorizedVoidMemberFunction0 {
op_precompute<Op>::apply(len);
VectorizedVoidOperation0<Op,class_type> vop(cls);
dispatchTask(vop,len);
mathexcon.handleOutstandingExceptions();
PY_IMATH_RETURN_PYTHON;
return cls;
}
};
Expand All @@ -704,7 +704,7 @@ struct VectorizedVoidMemberFunction1 {
op_precompute<Op>::apply(len);
VectorizedVoidOperation1<Op,class_type,arg1_type> vop(cls,arg1);
dispatchTask(vop,len);
mathexcon.handleOutstandingExceptions();
PY_IMATH_RETURN_PYTHON;
return cls;
}

Expand Down Expand Up @@ -751,7 +751,7 @@ struct VectorizedVoidMaskableMemberFunction1 {
dispatchTask(vop,len);
}

mathexcon.handleOutstandingExceptions();
PY_IMATH_RETURN_PYTHON;
return cls;
}

Expand Down Expand Up @@ -781,7 +781,7 @@ struct VectorizedVoidMemberFunction2 {
op_precompute<Op>::apply(len);
VectorizedVoidOperation2<Op,class_type,arg1_type,arg2_type> vop(cls,arg1,arg2);
dispatchTask(vop,len);
mathexcon.handleOutstandingExceptions();
PY_IMATH_RETURN_PYTHON;
return cls;
}

Expand Down Expand Up @@ -812,7 +812,7 @@ struct VectorizedMemberFunction0 {
result_type retval = create_uninitalized_return_value<result_type>::apply(len);
VectorizedOperation1<Op,result_type,class_type> vop(retval,cls);
dispatchTask(vop,len);
mathexcon.handleOutstandingExceptions();
PY_IMATH_RETURN_PYTHON;
return retval;
}
};
Expand All @@ -836,7 +836,7 @@ struct VectorizedMemberFunction1 {
result_type retval = create_uninitalized_return_value<result_type>::apply(len);
VectorizedOperation2<Op,result_type,class_type,arg1_type> vop(retval,cls,arg1);
dispatchTask(vop,len);
mathexcon.handleOutstandingExceptions();
PY_IMATH_RETURN_PYTHON;
return retval;
}

Expand Down Expand Up @@ -868,7 +868,7 @@ struct VectorizedMemberFunction2 {
result_type retval = create_uninitalized_return_value<result_type>::apply(len);
VectorizedOperation3<Op,result_type,class_type,arg1_type,arg2_type> vop(retval,cls,arg1,arg2);
dispatchTask(vop,len);
mathexcon.handleOutstandingExceptions();
PY_IMATH_RETURN_PYTHON;
return retval;
}

Expand Down
9 changes: 8 additions & 1 deletion PyIlmBase/PyImath/PyImathFixedArray.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,18 @@
#include <iostream>
#include <IexMathFloatExc.h>
#include "PyImathUtil.h"
#include <PyIlmBaseConfigInternal.h>

#define PY_IMATH_LEAVE_PYTHON IEX_NAMESPACE::MathExcOn mathexcon (IEX_NAMESPACE::IEEE_OVERFLOW | \
#ifdef PYIMATH_ENABLE_EXCEPTIONS
# define PY_IMATH_LEAVE_PYTHON IEX_NAMESPACE::MathExcOn mathexcon (IEX_NAMESPACE::IEEE_OVERFLOW | \
IEX_NAMESPACE::IEEE_DIVZERO | \
IEX_NAMESPACE::IEEE_INVALID); \
PyImath::PyReleaseLock pyunlock;
# define PY_IMATH_RETURN_PYTHON mathexcon.handleOutstandingExceptions()
#else
# define PY_IMATH_LEAVE_PYTHON PyImath::PyReleaseLock pyunlock;
# define PY_IMATH_RETURN_PYTHON
#endif

namespace PyImath {

Expand Down
7 changes: 6 additions & 1 deletion PyIlmBase/PyImath/PyImathMathExc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,14 @@
#define _PyImathMathExc_h_

#include <IexMathFloatExc.h>
#include <PyIlmBaseConfigInternal.h>

#define MATH_EXC_ON IEX_NAMESPACE::MathExcOn mathexcon (IEX_NAMESPACE::IEEE_OVERFLOW | \
#ifdef PYIMATH_ENABLE_EXCEPTIONS
# define MATH_EXC_ON IEX_NAMESPACE::MathExcOn mathexcon (IEX_NAMESPACE::IEEE_OVERFLOW | \
IEX_NAMESPACE::IEEE_DIVZERO | \
IEX_NAMESPACE::IEEE_INVALID)
#else
# define MATH_EXC_ON
#endif

#endif
6 changes: 6 additions & 0 deletions PyIlmBase/config/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@
### The autoconf setup for this folder generates a PyIlmBaseConfig.h file
### but no source actually uses that, so let's elide that for now

configure_file(PyIlmBaseConfigInternal.h.in_cmake ${CMAKE_CURRENT_BINARY_DIR}/PyIlmBaseConfigInternal.h)
add_library(PyIlmBaseConfig INTERFACE)
target_include_directories(PyIlmBaseConfig INTERFACE
$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>)
add_library(PyIlmBase::Config ALIAS PyIlmBaseConfig)

# The main export of the configuration - This is the
# moral equivalent of a pkg-config file for cmake
# and replaces the Find*.cmake of the "old" cmake
Expand Down
2 changes: 1 addition & 1 deletion PyIlmBase/config/ModuleDefine.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ function(PYILMBASE_DEFINE_MODULE modname)
list(APPEND libarglist CURDIR ${PYILMBASE_CURMOD_CURDIR})
endif()
if(PYILMBASE_CURMOD_CURBINDIR)
list(APPEND libarglist CURDIR ${PYILMBASE_CURMOD_CURBINDIR})
list(APPEND libarglist CURBINDIR ${PYILMBASE_CURMOD_CURBINDIR})
endif()
if(PYILMBASE_CURMOD_DEPENDENCIES)
list(APPEND libarglist DEPENDENCIES ${PYILMBASE_CURMOD_DEPENDENCIES})
Expand Down
11 changes: 11 additions & 0 deletions PyIlmBase/config/PyIlmBaseConfigInternal.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//
// SPDX-License-Identifier: BSD-3-Clause
// Copyright Contributors to the OpenEXR Project.
//

#pragma once

//
// Dealing with FPEs
//
#undef PYIMATH_ENABLE_EXCEPTIONS
9 changes: 9 additions & 0 deletions PyIlmBase/config/PyIlmBaseConfigInternal.h.in_cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// SPDX-License-Identifier: BSD-3-Clause
// Copyright Contributors to the OpenEXR Project.

#pragma once

//
// Dealing with FPEs
//
#cmakedefine PYIMATH_ENABLE_EXCEPTIONS 0
4 changes: 4 additions & 0 deletions PyIlmBase/config/PyIlmBaseSetup.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ include(GNUInstallDirs)
set(PYILMBASE_OVERRIDE_PYTHON2_INSTALL_DIR "" CACHE STRING "Override the install location for any python 2.x modules compiled")
set(PYILMBASE_OVERRIDE_PYTHON3_INSTALL_DIR "" CACHE STRING "Override the install location for any python 3.x modules compiled")

# Enables tracking of floating point exceptions and throwing them
# as the signals are received
option(PYIMATH_ENABLE_EXCEPTIONS "Enable runtime floating point exceptions" OFF)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I reading this correctly - the new option is off by default, is that the intention? Previous to this change, fp exceptions would be handled, but now they're only handled if you specify this option? Seems it would be better to leave the behavior as is, but provide an option to disable it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, you are reading the change correctly. As near as I can tell, the fp exception mechanism only seems reliable for gcc under linux, so I am suggesting disabling it by default. We could leave it on for linux / gcc by default. Even with clang, it doesn't seeom work reliably under linux. But we're also doing an incredible amount of work for every single operation in pyimath (i.e. setting and clearing the exception handler), it seemed ridiculous. We can talk about it at the meeting this week? Maybe there's something I'm missing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'm fine with changing the default, consistency is better.


# What C++ standard to compile for
# VFX Platform 18 is c++14, so let's enable that by default
set(tmp 14)
Expand Down