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

Move the dBigVector pair into a named structure. #244

Closed
wants to merge 6 commits into from
Closed

Move the dBigVector pair into a named structure. #244

wants to merge 6 commits into from

Conversation

iSLC
Copy link
Contributor

@iSLC iSLC commented Aug 7, 2021

Because dBigVector does not have a trivial constructor it must be moved into a named structure before it can be used like this.

Anonymous unions and structures are a compiler extension. Some compilers have more relaxed rules regarding this behavior (such as Clang or MSVC) because it can lead to undesired results. Whereas other compilers (such as GCC) are more strict and disallow code that can potentially lead to undefined behavior.

It may look a bit weird having it like this and a bit out of place but there's no other way to keep it as close to initial approach while still respecting compiler rules.

Should fix las issue in #241

@JulioJerez
Copy link
Contributor

ok I see the pull request by although that will work, we nee to make the class complete define all teh constructors and force the to be inline. not doing that and copy constructors and the other will resolve to a memcopy. and we do not what that
it is possible that CGG and Clang, and maybe even later VS are smart enough to no issuing the rep move but I knwo for a fact that VS 15 and 17 will issues ton of rep move and that will no be good. that class is use heavilly by teh joint solver, so it better be very effiecnt.

so this class is impmented like this

   struct Bisect
    {
        D_INLINE Bisect() = default;
        D_INLINE Bisect(const dBigVector & l, const dBigVector & h)
            :low(l), high(h)
        {
        }

        dBigVector low;
        dBigVector high;
    };

is implemented as

	struct ndData
	{
		D_INLINE ndData(const dFloat64 a)
			:m_low(a)
			,m_high(a)
		{
		}

		D_INLINE ndData(const ndData& data)
			:m_low(data.m_low)
			,m_high(data.m_high)
		{
		}

		D_INLINE ndData(const dBigVector& low, const dBigVector& high)
			:m_low(low)
			,m_high(high)
		{
		}

		dBigVector m_low;
		dBigVector m_high;
	};

@JulioJerez
Copy link
Contributor

please sync when you have time and tell me if is works now.

@iSLC
Copy link
Contributor Author

iSLC commented Aug 8, 2021 •
edited
Loading

Off-topic question. Did you remove the CMake code that adds the -mavx2 -mfma to compiler? Earlier when I tried to compile I got compiler errors that AVX and FMA instructions could not be used and when I looked the CMake code that added those compiler flags was not there anymore.

@iSLC iSLC closed this Aug 8, 2021
@JulioJerez
Copy link
Contributor

JulioJerez commented Aug 8, 2021 via email

@JulioJerez
Copy link
Contributor

JulioJerez commented Aug 8, 2021 via email

@JulioJerez
Copy link
Contributor

JulioJerez commented Aug 8, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@iSLC @JulioJerez