Properly tag non integer constants as unsigned int

[nbhuiyan]

Changes include:

• Interpreter pushes string into stack as OM::Value of uint48 kind
• INT_JMP bytecodes made into generic non type-specific JMP bytecodes
• New methods in MethodBuilder: push/popUint48() meant to handle strings
• b9_prim_print_string now expects a uint48 value for string index
• Fixed a potential bug in the frontend where the variable comparator was being declared as global variable due to missing var keyword
• Removed a helper method in the interpreter test that returns a string literal
• Ran clang-format
• Updated b9 assembly example with the new equivalent jmp operator

Signed-off-by: Nazim Uddin Bhuiyan nazimudd@ualberta.ca

[nbhuiyan]

Build will not pass due to missing inline methods in OMR::Om::ValueBuilder:

inline TR::IlValue* getUint48(TR::IlBuilder* b, TR::IlValue* v) {
    return b->And(v, b->ConstInt64(Om::Value::PAYLOAD_MASK));
}

inline TR::IlValue* fromUint48(TR::IlBuilder* b, TR::IlValue* i) {
    return b->Or(b->And(i, b->ConstInt64(Value::PAYLOAD_MASK)),
                 b->ConstInt64(Om::Value::Tag::UINT48));
}

[rwy7]

I don't think you need a special set of operations for string comparison--you should be able to just compare the tagged values.

[dnakamura]

I don't think you need a special set of operations for string comparison--you should be able to just compare the tagged values.

Unless you want to catch the case of differing string constants, pointing to identical text

[rwy7]

That's a different case though. For "string objects", you would see a GC reference on the stack, and use some kind of dynamic dispatch to compare.

In this case, we're only working with "symbols", which are special immediates corresponding to interned strings. With symbols, it should be impossible to have two different "identities" with the same "value".

[nbhuiyan]

@rwy0717 Yup, I could just make use of Om::Value's comparison operators. However, I saw that we had STR_JMP_EQ and STR_JMP_NEQ being present but not being used/implemented, so thought that it was perhaps appropriate making use of them instead of having INT_JMP_EQ handle string comparisons.

What do you think we can do about the STR_JMP_EQ and STR_JMP_NEQ bytecodes? Should the frontend changes that push INT/STR_JMP_* based on whether it's an int or str comparison remain? We can then have STR_JMP_EQ/NEQ fall through to INT_JMP_EQ/NEQ handlers, and just compare the tagged Values.

[rwy7]

One of the choices we've made in b9 is that the bytecode operations are untyped, and their behaviour depends on the types of the operands during execution. In the front end, you will often not know which comparison to use, especially in the case of dynamically typed variables or properties.

Consider:

a[x] == b[x];

We cannot know the types we are comparing until runtime.

I would like to add that some operations cannot operate on every type of operand. Many should throw an exception on an unexpected operand type. EG: bitwise-and should throw if the operand is not an integer. For now, we can just crash. I would like to see this PR add many more type assertions into the interpreter, now that we are introducing more types.

I also want to remind you that while eq and ne can be implemented in constant-time for symbols, ordered comparison will have to compare the underlying interned strings. We may have to stringify some integer operands as well.

For what it's worth, the document listing our future operators is here.

[rwy7]

I don't think it makes sense to have int/str comparison alias either, I think it's just confusing. You should just replace the int/str compare bytecodes with one set of untyped operators.

Also, while bytecodes are not typed, values are. That means that the push-constant bytecodes should continue to be typed.

[nbhuiyan]

@rwy0717 ready for review

[rwy7]

Looks good, it's almost ready to go. I committed the builder helpers to om last night, so once my last comment is addressed we'll accept. Great work as usual :D

[nbhuiyan]

Thanks @rwy0717 ! I've addressed your last comment. I agree that we should not be using bytecode handlers as helpers.