C++: Fix node types

[MathiasVP]

This PR fixes two classes of bugs related to computing the type of a DataFlow::Node:

• The type of Operands and Instructions weren't using the dataflow specific way to get the type of those entities. In certain cases the IR assigns some instructions an Unknown type (for soundness reasons, I guess?) whereas there's a more intuitive type to assign to the instruction for dataflow-purposes. We were already doing the correct thing for SSA, but we weren't doing so in the dataflow files.
• The type of an InitialGlobalValue node's at indirection index 0 was (e.g.) int instead of int*. It should be int* because its the InitialGlobalValue at indirection index 1 that represents the value pointed to by the global variable.

The first commit adds a new test that reveals these problems. The second commits fixes the issues, and the third commit accepts the test changes (which are all good 🎉).

[jketema]

Since we have type = globalDef.getUnspecifiedType(), I think the following should do.

Suggested change

	if this.isGLValue()
	then result = globalDef.getUnspecifiedType()
	else result = getTypeImpl(type.getUnspecifiedType(), globalDef.getIndirectionIndex() - 1)
	if this.isGLValue()
	then result = type
	else result = getTypeImpl(type, globalDef.getIndirectionIndex() - 1)

[jketema]

What I don't understand is why here we do not recurse in the case of a glvalue, while in the two case below we do.

[MathiasVP]

Since we have type = globalDef.getUnspecifiedType(), I think the following should do.

Agreed. Thanks for that!

[jketema]

What I don't understand is why here we do not recurse in the case of a glvalue, while in the two case below we do.

This is still unanswered 😄

[MathiasVP]

Yep. I'm trying to formulate a good response to this right now 😂.

[jketema]

Take your time. I thought it might have slipped through, but apparently it didn't.

[MathiasVP]

What I don't understand is why here we do not recurse in the case of a glvalue, while in the two case below we do.

Yeah, that's a bit unfortunate. I couldn't quite make it fit the pattern as the other cases. The reason is that, the other cases (i.e., IndirectOperand and IndirectInstruction) are defined with indirection indices in the range 1 to some max number.

However, the GlobalDef that defined an InitialGlobalValue has an index that's in the range 0 to some max number. And that discrepancy between the ranges meant I couldn't figure out how to write it in the same style as the other two cases 😕.

It may be that the real fix is to modify the indices of InitialGlobalValue so that its indirection index range starts 1 instead of 0 🤔. However, I'd like to delay such a larger change to another PR if you're okay with it. I can also add a version of the above explanation to a comment in this predicate if you think that's a good idea.

[jketema]

So, isGLValue can only be true is the index has the minimum value?

[MathiasVP]

Short answer: Yes.

Longer answer:
Note that isGLValue is only implemented for InstructionNodes, OperandNodes and InitialGlobalValue.InstructionNode and OperandNode (which intuitively can be seen as IndirectInstructionNodes and IndirectOperandNodes with indirection index 0) implement the predicate by checking if the underlying operand or instruction is a glvalue. Similarly, InitialGlobalValue implements the glvalue check by checking if the indirection index is 0.

Does that make sense? I'd be happy to do a Zoom chat about it if that's any easier.

[MathiasVP]

DCA is uneventful (as expected).

[jketema]

LGTM