[Insight-developers] Rounding functions in itkMacro.h

Luis Ibanez luis.ibanez at kitware.com
Sat May 16 09:09:15 EDT 2009 

Hi Tom

Thanks for uploading the new version of the patch.

I'm first checking that everything will work if we have
ITK_USE_PORTABLE_ROUND set to OFF. Then we
can enable a couple of machines to deal with the ON case.

So, for the OFF case:

A) After applying it (locally) three test were failing.

    1)  itkInterpolateTest
    2)  ResampleImageFilter3Test1
    3)  ResampleImageFilter9Test

    The first one has been fixed. It had hand-coded
     values that implied the rounding from continuous
    indexes to integers.

    The two others seems to require new baseline images,
    although it is strange that they are the only ones failing.

B) In order to use ITK from an external project I had to change

 SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -msse2")

      to

 SET(ITK_REQUIRED_CXX_FLAGS "${ITK_REQUIRED_CXX_FLAGS} -msse2")


I'm looking at the two remaining failing tests,
and plan to commit your patch by the end of the morning.



     Thanks


           Luis



-------------------------------------------------------------------------------------------
On Fri, May 15, 2009 at 1:05 PM, Tom Vercauteren
<tom.vercauteren at m4x.org> wrote:
> Hi Luis,
>
> I have posted a new version of the portable round patch on the bug tracker:
> http://public.kitware.com/Bug/view.php?id=6558
> http://public.kitware.com/Bug/file_download.php?file_id=2254&type=bug
>
> It fixes the include problem that I mentioned and also modifies two things:
> 1) VNL_CONFIG_ENABLE_SSE2_ROUNDING is turned ON by default only if the
> compiler supports it natively (i.e. without the -msse2 flag)
> 2) VNL_CONFIG_ENABLE_SSE2_ROUNDING is exported in ITKConfig.cmake.in
>
> Both modification were done to improve the user experience when using
> ITK from an external project.
>
> Regarding point 1:
> My previous patch was setting VNL_CONFIG_ENABLE_SSE2_ROUNDING to ON by
> default if the compiler supported sse2 natively OR if it supported it
> with the -msse2 flag. If -msse2 was required, cmake was adding this
> flag by default.
> This is not very nice to the user as it requires him by default to
> also use this flag in its external project even though he didn't
> explicitly ask to add sse2 support.
>
> Regarding point 2:
> If the user explicitly chose to turn  VNL_CONFIG_ENABLE_SSE2_ROUNDING
> and his compiler needs  the -mss2 flag for that, ITK will use it.
> That's fine. Now if ITK is included in an external project, this
> project will also require the -msse2 flag. Adding
> VNL_CONFIG_ENABLE_SSE2_ROUNDING to  ITKConfig.cmake allows the
> external project to know whether it needs to use -msse2 by default or
> not.
>
> Cheers,
> Tom
>
>
> On Wed, May 13, 2009 at 01:05, Luis Ibanez <luis.ibanez at kitware.com> wrote:
>>
>> Tom,
>>
>> Thanks for catching the problem.
>>
>> I'll try running another Experimental tonight.
>>
>>
>>   Luis
>>
>>
>> ------------------------
>> Tom Vercauteren wrote:
>>>
>>> Oups, in my previous email
>>>  "I see that vnl_math.h is only included if ITK_USE_PORTABLE_ROUND is ON."
>>> should have been
>>>  "I see that vnl_math.h is only included if ITK_USE_PORTABLE_ROUND is
>>> OFF or not defined."
>>>
>>> Tom
>>>
>>> On Tue, May 12, 2009 at 23:26, Tom Vercauteren <tom.vercauteren at m4x.org>
>>> wrote:
>>>
>>>> Hi Luis,
>>>>
>>>> I think I got the culprit. It didn't make sense to me how
>>>> vnl_math_rnd_halfintup could be undeclared. I am using ubuntu 9.04
>>>> with gcc 4.3 and every thing works out fine. Se for example this debug
>>>> build with sse2 rounding:
>>>> http://www.cdash.org/CDash/buildSummary.php?buildid=331407
>>>> http://www.cdash.org/CDash/testDetails.php?test=22870870&build=331407
>>>>
>>>> The only possible reason for a mising vnl_math_rnd_halfintup is a
>>>> missing or outdated vnl_math.h. Looking back at the code
>>>>
>>>> http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/Common/itkMacro.h?root=Insight&view=markup
>>>> I see that vnl_math.h is only included if ITK_USE_PORTABLE_ROUND is
>>>> ON. I wasn't using this variable...
>>>>
>>>> Could you please try and recompile by changing
>>>> #ifndef ITK_USE_PORTABLE_ROUND
>>>> #include "vnl/vnl_math.h"
>>>> #endif
>>>> by
>>>> #include "vnl/vnl_math.h"
>>>> in
>>>> itkMacro.h
>>>> ?
>>>>
>>>> Sorry for not giving you a patch but I don't have a development machine
>>>> at hand.
>>>>
>>>> Also, I saw from your previous email that you had set
>>>> VNL_CONFIG_ENABLE_SSE2 to ON. This is not used by the sse2 rounding
>>>> functions which only relies on VNL_CONFIG_ENABLE_SSE2_ROUNDING. Since
>>>> VXL developers aresaying that VNL_CONFIG_ENABLE_SSE2 is unstable, I
>>>> wouldn't recommend turning this option on.
>>>>
>>>> Hope this helps,
>>>> Tom
>>>>
>>>>
>>>> On Tue, May 12, 2009 at 18:14, Luis Ibanez <luis.ibanez at kitware.com>
>>>> wrote:
>>>>
>>>>> HI Tom,
>>>>>
>>>>> Thanks for the new patch.
>>>>>
>>>>> Unfortunately it is not compiling for me.
>>>>>
>>>>>
>>>>> Here is what I got:
>>>>>
>>>>>
>>>>> [ 41%] Building CXX object
>>>>> Code/Common/CMakeFiles/ITKCommon.dir/itkBarrier.o
>>>>> In file included from
>>>>> /home/ibanez/src/Insight/Code/Common/itkTimeStamp.h:23,
>>>>>               from
>>>>> /home/ibanez/src/Insight/Code/Common/itkLightObject.h:21,
>>>>>               from /home/ibanez/src/Insight/Code/Common/itkBarrier.h:20,
>>>>>               from
>>>>> /home/ibanez/src/Insight/Code/Common/itkBarrier.cxx:17:
>>>>> /home/ibanez/src/Insight/Code/Common/itkMacro.h: In function ‘int
>>>>> itk::Math::RoundHalfIntegerUp(float)’:
>>>>> /home/ibanez/src/Insight/Code/Common/itkMacro.h:979: error:
>>>>> ‘vnl_math_rnd_halfintup’ was not declared in this scope
>>>>> /home/ibanez/src/Insight/Code/Common/itkMacro.h: In function ‘int
>>>>> itk::Math::RoundHalfIntegerUp(double)’:
>>>>> /home/ibanez/src/Insight/Code/Common/itkMacro.h:980: error:
>>>>> ‘vnl_math_rnd_halfintup’ was not declared in this scope
>>>>> /home/ibanez/src/Insight/Code/Common/itkMacro.h: In function ‘int
>>>>> itk::Math::RoundHalfIntegerToEven(float)’:
>>>>> /home/ibanez/src/Insight/Code/Common/itkMacro.h:981: error:
>>>>> ‘vnl_math_rnd_halfinttoeven’ was not declared in this scope
>>>>> /home/ibanez/src/Insight/Code/Common/itkMacro.h: In function ‘int
>>>>> itk::Math::RoundHalfIntegerToEven(double)’:
>>>>> /home/ibanez/src/Insight/Code/Common/itkMacro.h:982: error:
>>>>> ‘vnl_math_rnd_halfinttoeven’ was not declared in this scope
>>>>> make[2]: *** [Code/Common/CMakeFiles/ITKCommon.dir/itkBarrier.o] Error 1
>>>>> make[1]: *** [Code/Common/CMakeFiles/ITKCommon.dir/all] Error 2
>>>>>
>>>>>
>>>>>
>>>>> and the SSE flags in my build are ON
>>>>>
>>>>> grep ENABLE_SSE MakeCache.txt
>>>>>
>>>>> VNL_CONFIG_ENABLE_SSE2:BOOL=ON
>>>>> VNL_CONFIG_ENABLE_SSE2_ROUNDING:BOOL=ON
>>>>> //Advanced flag for variable: VNL_CONFIG_ENABLE_SSE2
>>>>> VNL_CONFIG_ENABLE_SSE2-ADVANCED:INTERNAL=1
>>>>> //Modified flag for variable: VNL_CONFIG_ENABLE_SSE2
>>>>> VNL_CONFIG_ENABLE_SSE2-MODIFIED:INTERNAL=1
>>>>> //Advanced flag for variable: VNL_CONFIG_ENABLE_SSE2_ROUNDING
>>>>> VNL_CONFIG_ENABLE_SSE2_ROUNDING-ADVANCED:INTERNAL=1
>>>>> //Modified flag for variable: VNL_CONFIG_ENABLE_SSE2_ROUNDING
>>>>> VNL_CONFIG_ENABLE_SSE2_ROUNDING-MODIFIED:INTERNAL=1
>>>>>
>>>>>
>>>>> BTW: This is a Debug build in gcc 4.3.2, Linux Ubuntu 8.10.
>>>>>
>>>>>
>>>>>    Luis
>>>>>
>>>>>
>>>>> =========================================
>>>>> On Tue, May 12, 2009 at 8:48 AM, Tom Vercauteren
>>>>> <tom.vercauteren at m4x.org> wrote:
>>>>>
>>>>>> Luis,
>>>>>>
>>>>>> I have put an updated patch on the bug tracker:
>>>>>> http://public.kitware.com/Bug/file_download.php?file_id=2236&type=bug
>>>>>>
>>>>>> In addition to the changes I mentioned earlier, it includes some small
>>>>>> modifications to some CMakeLists.txt in order to ease turning on SSE2
>>>>>> rounding.
>>>>>> cmake -DVNL_CONFIG_ENABLE_SSE2_ROUNDING=ON <path_to_itk>
>>>>>> and
>>>>>> ccmake <path_to_itk>
>>>>>> switch VNL_CONFIG_ENABLE_SSE2_ROUNDING to on
>>>>>> now also works with gcc
>>>>>>
>>>>>> Let me know if it works for you.
>>>>>>
>>>>>> Tom
>>>>>>
>>>>>> On Tue, May 12, 2009 at 12:43, Tom Vercauteren
>>>>>> <tom.vercauteren at m4x.org> wrote:
>>>>>>
>>>>>>> Hi Luis,
>>>>>>>
>>>>>>> I have almost the same system so this will ease the process.
>>>>>>>
>>>>>>> The problem is indeed related to the -msse2 flag. Running
>>>>>>> cmake -DCMAKE_C_FLAGS="-msse2" -DCMAKE_CXX_FLAGS="-msse2"
>>>>>>> -DVNL_CONFIG_ENABLE_SSE2_ROUNDING=ON <path_to_itk>
>>>>>>> works correctly.
>>>>>>>
>>>>>>> So it looks like the cmake magic used by vxl to detect and add sse2
>>>>>>> support is wrong. Actually running
>>>>>>> cmake -DVNL_CONFIG_ENABLE_SSE2_ROUNDING=ON <path_to_itk>
>>>>>>> leads to a cmake error.
>>>>>>>
>>>>>>> This is not specific to the patched itk version as the same behavior
>>>>>>> is observed on a pristine checkout of the vxl svn trunk.
>>>>>>>
>>>>>>> There are at least two errors I can see from the cmake scripts in vxl
>>>>>>> svn trunk when VNL_CONFIG_ENABLE_SSE2_ROUNDING is ON.
>>>>>>>
>>>>>>> 1)  ADD_DEFINITIONS( -msse2 ) is called after vxl tries to detect
>>>>>>> whether it has sse2 support. However this flag is required for gcc
>>>>>>> (4.3) to find emmintrin.h and use the sse2 intrinsics.
>>>>>>>
>>>>>>> 2) Even if ADD_DEFINITIONS( -msse2 ) was called prior to
>>>>>>> PERFORM_CHECK_HEADER(emmintrin.h VXL_HAS_EMMINTRIN_H)
>>>>>>> and
>>>>>>> PERFORM_CMAKE_TEST_RUN(vxl_platform_tests.cxx
>>>>>>> VXL_HAS_SSE2_HARDWARE_SUPPORT)
>>>>>>> it wouldn't make a difference since PERFORM_CHECK_HEADER and
>>>>>>> PERFORM_CMAKE_TEST_RUN do not rely on such ADD_DEFINITIONS.
>>>>>>>
>>>>>>> I am not sure how to fix this right now as I am not a cmake expert.
>>>>>>> Any help would be appreciated.
>>>>>>>
>>>>>>>
>>>>>>> Note this this issue does not affect the case when
>>>>>>> VNL_CONFIG_ENABLE_SSE2_ROUNDING is OFF. In such a case, on your
>>>>>>> system, the implementation will fallback to the gcc assembly calls,
>>>>>>> e.g.
>>>>>>> inline int vnl_math_rnd_halfinttoeven(float  x) { int r; __asm__
>>>>>>> __volatile__ ("fistpl %0" : "=m"(r) : "t"(x) : "st"); return r; }
>>>>>>>
>>>>>>> I'll report back if I find a patch for the cmake scripts that I find
>>>>>>> reasonable.
>>>>>>>
>>>>>>> Tom
>>>>>>>
>>>>>>> On Tue, May 12, 2009 at 04:27, Luis Ibanez <luis.ibanez at kitware.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi Tom,
>>>>>>>>
>>>>>>>> I'm testing this on a Laptop:
>>>>>>>>
>>>>>>>>  Linux Ubuntu 8.10
>>>>>>>>  gcc 4.3.2
>>>>>>>>  Intel(R) Core(TM)2 Duo CPU     T9600  @ 2.80GHz
>>>>>>>>
>>>>>>>>
>>>>>>>> The error was a link error.
>>>>>>>> Missing symbols.
>>>>>>>>
>>>>>>>> lshw returns the following data from the CPU:
>>>>>>>>
>>>>>>>> *-cpu
>>>>>>>>        description: CPU
>>>>>>>>        product: Intel(R) Core(TM)2 Duo CPU     T9600  @ 2.80GHz
>>>>>>>>        vendor: Intel Corp.
>>>>>>>>        physical id: 400
>>>>>>>>        bus info: cpu at 0
>>>>>>>>        version: 6.7.6
>>>>>>>>        slot: Microprocessor
>>>>>>>>        size: 2801MHz
>>>>>>>>        capacity: 2801MHz
>>>>>>>>        width: 64 bits
>>>>>>>>        clock: 266MHz
>>>>>>>>        capabilities: fpu fpu_exception wp vme de pse tsc msr pae
>>>>>>>> mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx
>>>>>>>> fxsr
>>>>>>>> sse sse2 ss ht tm pbe nx x86-64 constant_tsc arch_perfmon pebs bts
>>>>>>>> pni
>>>>>>>> monitor ds_cpl vmx smx est tm2 ssse3 cx16 xtpr sse4_1 lahf_lm cpufreq
>>>>>>>>        configuration: id=0
>>>>>>>>
>>>>>>>> --
>>>>>>>>
>>>>>>>>    Luis
>>>>>>>>
>>>>>>>>
>>>>>>>> ------------------------------
>>>>>>>> On Mon, May 11, 2009 at 7:16 PM, Tom Vercauteren
>>>>>>>> <tom.vercauteren at m4x.org> wrote:
>>>>>>>>
>>>>>>>>> Luis,
>>>>>>>>>
>>>>>>>>> Can you provide some information on your configuration (OS,
>>>>>>>>> compiler,
>>>>>>>>> cpu)? Also did you get a compilation error or a link error?
>>>>>>>>>
>>>>>>>>> One potential thing to look at is whether your machine actually
>>>>>>>>> supports sse2 and whether vxl correctly detects it. It looks like I
>>>>>>>>> missed a patch to the sse2 detection in the list I gave you:
>>>>>>>>>
>>>>>>>>> http://vxl.svn.sourceforge.net/viewvc/vxl/trunk/config/cmake/config/CMakeLists.txt?r1=23457&r2=23456&pathrev=23457
>>>>>>>>>
>>>>>>>>> http://vxl.svn.sourceforge.net/viewvc/vxl/trunk/config/cmake/config/vxl_platform_tests.cxx?r1=23457&r2=23456&pathrev=23457
>>>>>>>>>
>>>>>>>>> No matter this patch, setting  VNL_CONFIG_ENABLE_SSE2_ROUNDING to
>>>>>>>>> OFF
>>>>>>>>> should definitely not remove the declarations of the new rounding
>>>>>>>>> methods. It should simply fallback to another implementation than
>>>>>>>>> the
>>>>>>>>> SSE2 one (gcc-specific or msvc-specific or vanilla c).
>>>>>>>>>
>>>>>>>>> Since I don't have access to my computer right now, it's a bit
>>>>>>>>> difficult for me to provide you more information right now.
>>>>>>>>>
>>>>>>>>> Tom
>>>>>>>>>
>>>>>>>>> On Tue, May 12, 2009 at 00:44, Luis Ibanez <luis.ibanez at kitware.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Tom,
>>>>>>>>>>
>>>>>>>>>> Setting  VNL_CONFIG_ENABLE_SSE2_ROUNDING to OFF
>>>>>>>>>> removes the declarations of the new rounding methods in
>>>>>>>>>> vnl_math.h.
>>>>>>>>>>
>>>>>>>>>> I'm probably doing something wrong here.
>>>>>>>>>>
>>>>>>>>>> I have generated a patch with the changes and uploaded it
>>>>>>>>>> to the bug tracker:
>>>>>>>>>>
>>>>>>>>>> You will find it in:
>>>>>>>>>>  http://public.kitware.com/Bug/view.php?id=6558
>>>>>>>>>>
>>>>>>>>>> is the patch named:
>>>>>>>>>>
>>>>>>>>>>        PortableRound-May-11-2009.patch
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> If you have a chance,
>>>>>>>>>> could you help me find out what I'm missing ?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>    Thanks
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>          Luis
>>>>>>>>>>
>>>>>>>>
>>>
>>
>

More information about the Insight-developers mailing list