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.

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

1.1.7 release change removes gcc atomics on freebsd i386 #529

Closed
sambler opened this issue Feb 22, 2013 · 5 comments
Closed

1.1.7 release change removes gcc atomics on freebsd i386 #529

sambler opened this issue Feb 22, 2013 · 5 comments

Comments

@sambler
Copy link

sambler commented Feb 22, 2013

Thought I would discuss this as an issue before submitting a patch.

The recent change (v1.1.7) to re-disable gcc atomics on i386 freebsd undoes the changes I got committed to enable it. The change to the platform.cmake added -march=i586 to CXXFLAGS allowing gcc atomics to be used on a 32bit build, this change was needed as using tbb on i386 failed to create a runnable binary. (based on a blender build with oiio)

If you don't want the USE_TBB option to affect the use of gcc atomics then you should remove the nested conditional around the define USE_GCC_ATOMICS so that gcc atomics is enabled whenever it is available.

@lgritz
Copy link
Collaborator

lgritz commented Apr 16, 2013

What's the status of this? Is all ok now?

@sambler
Copy link
Author

sambler commented Apr 19, 2013

It hasn't been changed since. I believe lines 113 and 115 should be removed from src/include/thread.h Really a matter of you undoing the previous patch rather than removing the relevant if test.

@lgritz
Copy link
Collaborator

lgritz commented Jul 27, 2013

Hi, sorry, reviving this again. Is this currently broken in master? The thread.h file has changed a lot since this was filed and I don't know if it's working or not on BSD with i386.

@danwexler
Copy link
Contributor

Lemme know if you need me to submit any reviews on this stuff. I can't
remember the state of ARM atomics, but I can double check if it is causing
problems.

On Sat, Jul 27, 2013 at 2:43 PM, Larry Gritz notifications@github.comwrote:

Hi, sorry, reviving this again. Is this currently broken in master? The
thread.h file has changed a lot since this was filed and I don't know if
it's working or not on BSD with i386.


Reply to this email directly or view it on GitHubhttps://github.com//issues/529#issuecomment-21673065
.

@sambler
Copy link
Author

sambler commented Jul 31, 2013

Yes this is still unchanged. The define USE_GCC_ATOMICS currently at line 116 is surrounded by a FreeBSD test which isn't needed, just need the GNUC test. The solution for using gcc atomics on i386 is to use the -march=i586 CXXFLAG which has been added to platform.cmake. Now that I think of it the USE_TBB test should be removed from there as well. I just submitted a pull request 648 with these changes.

@lgritz lgritz closed this as completed Aug 5, 2013
splbio pushed a commit to splbio/freebsd-ports that referenced this issue Nov 24, 2014
The change to thread.h has been reported upstream in
AcademySoftwareFoundation/OpenImageIO#529.

PR:		ports/178505
Submitted by:	Shane Ambler <FreeBSD@Shaneware.biz> (maintainer)


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@317935 35697150-7ecd-e111-bb59-0022644237b5
svmhdvn pushed a commit to svmhdvn/freebsd-ports that referenced this issue Jan 10, 2024
The change to thread.h has been reported upstream in
AcademySoftwareFoundation/OpenImageIO#529.

PR:		ports/178505
Submitted by:	Shane Ambler <FreeBSD@Shaneware.biz> (maintainer)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants