Audio plugin host - weird behavior in when compiler optimization On (VS2017)

Windows
1

Hi,

Weird things happen. While playing around with the ‘audio plugin host’ demo solution under VS2017 (Win32) I noticed sth really weird. In Debug mode everything’s working fine. When building in Release mode, I am not able to connect pins anymore. My guess it’s the optimizer doing sth wrong (as it’s turned on in Relase mode). Of course I’m using everything “out of the box” - unmodified latest JUCE framework and solutions generated by the ProJucer.

I managed to identify the code responsible for this bug (GraphEditorPanel.cpp, line 830):

PinComponent* GraphEditorPanel::findPinAt (Point<float> pos) const
{
    for (auto* child : getChildren())
        if (auto* fc = dynamic_cast<FilterComponent*> (child))
            if (auto* pin = dynamic_cast<PinComponent*> (fc->getComponentAt (pos.toInt() - fc->getPosition())))
                return pin;

    return nullptr;
}

In Release mode the “return pin” statement never gets executed (when I inserted a break point on this statement, it’s automatically removed by VS upon running it). It means the optimizer assumes the condition (second if) is always 0/nullptr (false). There is an assigment, so it means it assumes a result of the dynamic_cast is always 0. I did some test and changed the code a little bit into:

PinComponent* GraphEditorPanel::findPinAt (Point<float> pos) const
{
	for (auto* child : getChildren())
		if (auto* fc = dynamic_cast<FilterComponent*> (child))
		{
			auto* pin = (fc->getComponentAt(pos.toInt() - fc->getPosition()));
			auto* pinC = dynamic_cast<PinComponent*>(pin);
			if (pinC) 
				return pinC;
		}

	return nullptr;
}

Now (in Release mode) it’s working fine.

But if I change it into

PinComponent* GraphEditorPanel::findPinAt (Point<float> pos) const
{
	for (auto* child : getChildren())
		if (auto* fc = dynamic_cast<FilterComponent*> (child))
		{
			auto* pin= dynamic_cast<PinComponent*>(fc->getComponentAt(pos.toInt() - fc->getPosition()));
			if (pin) 
				return pin;
		}

	return nullptr;
}

then the optimizer assumes “pin” is always nullptr again.

Can anyone explain this behavior? What makes such difference for the optimizer, so that

			auto* pin = (fc->getComponentAt(pos.toInt() - fc->getPosition()));
			auto* pinC = dynamic_cast<PinComponent*>(pin);
// optimizer doesn't make any assumptions regarding pin/pinC and everything is working fine

is treated so much different than

			auto* pin = dynamic_cast<PinComponent*>(fc->getComponentAt(pos.toInt() - fc->getPosition()));
// optimizer assumes "pin" will be always nullptr

I have a feeling that the optimizer should be blamed, as in the Debug mode when the optimizer is turned off, “pin” is not always nullptr! So the optimizer fails assuming it’s alwyas nullptr. It’s not really a JUCE question (and definitely not JUCE framework’s fault!), as the same behaviour might occur in any other C++ code containing similar operations.

(reminder: getComponentAt returns Component*, PinComponent is a subclass (though defined as a struct) of Component class).

2

Confirmed, this seems to happen with the latest VS 2017 Preview 2 also. (Just the release mode 32 bit build.)

3

If you look at the latest code on develop, that class has been completely rewritten! Give the new version a shot, and let me know if you still see any weirdness!

4

It happens with the latest revision too.

5

Yeah, I can see the code for the method has the same statement causing issues:

GraphEditorPanel::PinComponent* GraphEditorPanel::findPinAt (Point<float> pos) const
{
    for (auto* fc : nodes)
        if (auto* pin = dynamic_cast<PinComponent*> (fc->getComponentAt (pos.toInt() - fc->getPosition())))
            return pin;

    return nullptr;
}

@jules As without the compiler’s optimisation the code works just fine, I don’t think there’s anything wrong with the JUCE demo code (even the code from master branch). I already provided a simple “fix” in the original post. My goal was to understand if there’s a bug in the optimiser (a glance at the connect MS forum gives an impression it happens quite often…) or maybe there’s sth specific with classes dependencies in the demo project causing such (still incorrect) optimiser behaviour. You know - since now I used to trust things like code optimisers etc.:wink:

6

Thanks. Looking at it properly now, that does seem completely bonkers, and must be a compiler error as far as I can tell! I guess it must be attempting some kind of devirtualisation and just getting it all wrong, probably assuming that a component can never be cast to a PinComponent and optimising that away.

I’ll add your workaround, thanks for digging into it!

7

Semantically these two blocks should be equal:
1:

Component* pin = (fc->getComponentAt(pos.toInt() - fc->getPosition()));
PinComponent* pinC = dynamic_cast<PinComponent*>(pin);

2:
PinComponent* pinC = dynamic_cast<PinComponent*>(fc->getComponentAt(pos.toInt() - fc->getPosition()));

Effectively there’s is exactly the same casting!

8

Yep. Compiler bugs can be very odd!