Skip to content

Commit

Permalink
Fix VMRegister Adjust APIs
Browse files Browse the repository at this point in the history
JitBuilder math APIs like Add, Mul, etc. require that the left and
right operands have the same type or if the left is a pointer that
the right is an Int32 or Int64. Currently a VMRegister wrapping a
int8_t * will fail when Adjust is called since
_integerTypeForAdjustment will be Int8. It is incorrect to limit
the adjustBy amount to the primite type of the pointer since a user
may want to adjust the value by a large value. The solution is to
adjust by an Int64 not matter what the primitive register type is.

This PR includes a new test called vmregister which verifies that
VMRegisters wrapping a pInt8 work. This test failed before this
PR. The Operand[Array,Stack]Tests verify that the code continues
to work for larger types.

Signed-off-by: Charlie Gracie <charlie.gracie@gmail.com>
  • Loading branch information
charliegracie committed Apr 17, 2019
1 parent a7063e5 commit 02678cf
Show file tree
Hide file tree
Showing 7 changed files with 186 additions and 19 deletions.
16 changes: 8 additions & 8 deletions compiler/ilgen/OMRVirtualMachineRegister.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2016, 2018 IBM Corp. and others
* Copyright (c) 2016, 2019 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -39,17 +39,17 @@ OMR::VirtualMachineRegister::VirtualMachineRegister(
"VirtualMachineRegister for %s requires pointerToRegisterType (%s) to be a pointer",
localName, pointerToRegisterType->getName());

_integerTypeForAdjustments = pointerToRegisterType->baseType();
if (_integerTypeForAdjustments->isPointer())
TR::IlType *registerBaseType = pointerToRegisterType->baseType();
if (registerBaseType->isPointer())
{
_integerTypeForAdjustments = _integerTypeForAdjustments->baseType();
if (_integerTypeForAdjustments->isPointer())
registerBaseType = registerBaseType->baseType();
if (registerBaseType->isPointer())
{
_integerTypeForAdjustments = b->typeDictionary()->getWord();
registerBaseType = b->typeDictionary()->getWord();
}
TR_ASSERT(adjustByStep == _integerTypeForAdjustments->getSize(),
TR_ASSERT(adjustByStep == registerBaseType->getSize(),
"VirtualMachineRegister for %s adjustByStep (%u) != size represented by pointerToRegisterType (%u)",
localName, _adjustByStep, _integerTypeForAdjustments->getSize());
localName, _adjustByStep, registerBaseType->getSize());
_isAdjustable = true;
}
else
Expand Down
7 changes: 3 additions & 4 deletions compiler/ilgen/OMRVirtualMachineRegister.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2016, 2018 IBM Corp. and others
* Copyright (c) 2016, 2019 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -161,7 +161,7 @@ class VirtualMachineRegister : public TR::VirtualMachineState
virtual void Adjust(TR::IlBuilder *b, TR::IlValue *amount)
{
TR::IlValue *off=b->Mul(amount,
b-> ConstInteger(_integerTypeForAdjustments, _adjustByStep));
b-> ConstInt64(_adjustByStep));
adjust(b, off);
}

Expand All @@ -174,7 +174,7 @@ class VirtualMachineRegister : public TR::VirtualMachineState
*/
virtual void Adjust(TR::IlBuilder *b, int64_t amount)
{
adjust(b, b->ConstInteger(_integerTypeForAdjustments, amount * _adjustByStep));
adjust(b, b->ConstInt64(amount * _adjustByStep));
}

/**
Expand Down Expand Up @@ -206,7 +206,6 @@ class VirtualMachineRegister : public TR::VirtualMachineState
const char * const _localName;
TR::IlValue * _addressOfRegister;
TR::IlType * _pointerToRegisterType;
TR::IlType * _integerTypeForAdjustments;
uint32_t _adjustByStep;
bool _isAdjustable;

Expand Down
14 changes: 7 additions & 7 deletions compiler/ilgen/OMRVirtualMachineRegisterInStruct.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2016, 2018 IBM Corp. and others
* Copyright (c) 2016, 2019 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -71,16 +71,16 @@ class VirtualMachineRegisterInStruct : public TR::VirtualMachineRegister
_fieldName(fieldName),
_localNameHoldingStructAddress(localNameHoldingStructAddress)
{
_integerTypeForAdjustments = b->typeDictionary()->GetFieldType(structName, fieldName);
if (_integerTypeForAdjustments->isPointer())
TR::IlType *registerBaseType = b->typeDictionary()->GetFieldType(structName, fieldName);
if (registerBaseType->isPointer())
{
_integerTypeForAdjustments = _integerTypeForAdjustments->baseType();
if (_integerTypeForAdjustments->isPointer())
registerBaseType = registerBaseType->baseType();
if (registerBaseType->isPointer())
{
_integerTypeForAdjustments = b->typeDictionary()->getWord();
registerBaseType = b->typeDictionary()->getWord();
}
_isAdjustable = true;
_adjustByStep = _integerTypeForAdjustments->getSize();
_adjustByStep = registerBaseType->getSize();
}
else
{
Expand Down
1 change: 1 addition & 0 deletions jitbuilder/release/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ if(OMR_JITBUILDER_TEST_EXTENDED)
create_jitbuilder_test(switch cpp/samples/Switch.cpp)
create_jitbuilder_test(toiltype cpp/samples/ToIlType.cpp)
create_jitbuilder_test(union cpp/samples/Union.cpp)
create_jitbuilder_test(vmregister cpp/samples/VMRegister.cpp)
endif()

# Experimental Tests: These are still on the experimental side
Expand Down
8 changes: 8 additions & 0 deletions jitbuilder/release/cpp/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ ALL_TESTS = \
toiltype \
transactionaloperations \
union \
vmregister \
worklist \

# Compile all the tests by default
Expand Down Expand Up @@ -93,6 +94,7 @@ all_goal: common_goal
./thunks
./toiltype
./union
./vmregister

# Tests that are still on the experimental side
# If you add to this list and want them to be built under "make", please
Expand Down Expand Up @@ -294,6 +296,12 @@ union : $(LIBJITBUILDER) Union.o

Union.o: $(SAMPLE_SRC)/Union.cpp $(SAMPLE_SRC)/Union.hpp
$(CXX) -o $@ $(CXXFLAGS) $<

vmregister : $(LIBJITBUILDER) VMRegister.o
$(CXX) -g -fno-rtti -o $@ VMRegister.o -L$(LIBJITBUILDERDIR) -ljitbuilder -ldl

VMRegister.o: $(SAMPLE_SRC)/VMRegister.cpp $(SAMPLE_SRC)/VMRegister.hpp
$(CXX) -o $@ $(CXXFLAGS) $<

worklist : $(LIBJITBUILDER) Worklist.o
$(CXX) -g -fno-rtti -o $@ Worklist.o -L$(LIBJITBUILDERDIR) -ljitbuilder -ldl
Expand Down
123 changes: 123 additions & 0 deletions jitbuilder/release/cpp/samples/VMRegister.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/*******************************************************************************
* Copyright (c) 2016, 2019 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
* distribution and is available at https://www.eclipse.org/legal/epl-2.0/
* or the Apache License, Version 2.0 which accompanies this distribution and
* is available at https://www.apache.org/licenses/LICENSE-2.0.
*
* This Source Code may also be made available under the following
* Secondary Licenses when the conditions for such availability set
* forth in the Eclipse Public License, v. 2.0 are satisfied: GNU
* General Public License, version 2 with the GNU Classpath
* Exception [1] and GNU General Public License, version 2 with the
* OpenJDK Assembly Exception [2].
*
* [1] https://www.gnu.org/software/classpath/license.html
* [2] http://openjdk.java.net/legal/assembly-exception.html
*
* SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 OR LicenseRef-GPL-2.0 WITH Assembly-exception
*******************************************************************************/


#include <iostream>
#include <stdlib.h>
#include <stdint.h>
#include <errno.h>

#include "VMRegister.hpp"
#include "JitBuilder.hpp"

using std::cout;
using std::cerr;

#define TOSTR(x) #x
#define LINETOSTR(x) TOSTR(x)


int
main(int argc, char *argv[])
{
cout << "Step 1: initialize JIT\n";
bool initialized = initializeJit();
if (!initialized)
{
cerr << "FAIL: could not initialize JIT\n";
exit(-1);
}

cout << "Step 2: define type dictionary\n";
OMR::JitBuilder::TypeDictionary types;

cout << "Step 3: compile method builder\n";
VMRegisterMethod method(&types);
void *entry = 0;
int32_t rc = compileMethodBuilder(&method, &entry);
if (rc != 0)
{
cerr << "FAIL: compilation error " << rc << "\n";
exit(-2);
}

cout << "Step 4: invoke compiled code and print results\n";
typedef int32_t (VMRegisterMethodFunction)(int8_t **values, int32_t count);
VMRegisterMethodFunction *test = (VMRegisterMethodFunction *) entry;

int8_t values[] = {7,2,9,5,3,1,6};
int8_t *vals = values;
int32_t retVal = test(&vals, 7);
cout << "test(values) returned " << retVal << "\n";

cout << "Step 5: shutdown JIT\n";
shutdownJit();
}



VMRegisterMethod::VMRegisterMethod(OMR::JitBuilder::TypeDictionary *d)
: OMR::JitBuilder::MethodBuilder(d, (OMR::JitBuilder::VirtualMachineState *) NULL)
{
DefineLine(LINETOSTR(__LINE__));
DefineFile(__FILE__);

DefineName("test");
DefineParameter("valuesPtr", d->PointerTo(d->PointerTo(Int8)));
DefineParameter("count", Int32);
DefineReturnType(Int32);
}

bool
VMRegisterMethod::buildIL()
{
OMR::JitBuilder::TypeDictionary *dict = typeDictionary();
OMR::JitBuilder::IlType *pElementType = dict->PointerTo(Int8);
OMR::JitBuilder::IlType *ppElementType = dict->PointerTo(pElementType);
OMR::JitBuilder::VirtualMachineRegister *vmreg = new OMR::JitBuilder::VirtualMachineRegister(this, "MYBYTES", ppElementType, sizeof(int8_t), Load("valuesPtr"));

Store("result", ConstInt32(0));

OMR::JitBuilder::IlBuilder *loop = NULL;
ForLoopUp("i", &loop,
ConstInt32(0),
Load("count"),
ConstInt32(1));

loop->Store("valAddress",
vmreg->Load(loop));

loop->Store("val",
loop-> LoadAt(pElementType,
loop-> Load("valAddress")));

loop->Store("result",
loop-> Add(
loop-> Load("result"),
loop-> ConvertTo(Int32,
loop-> Load("val"))));
vmreg->Adjust(loop, 1);

Return(Load("result"));

return true;
}
36 changes: 36 additions & 0 deletions jitbuilder/release/cpp/samples/VMRegister.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*******************************************************************************
* Copyright (c) 2016, 2019 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
* distribution and is available at https://www.eclipse.org/legal/epl-2.0/
* or the Apache License, Version 2.0 which accompanies this distribution and
* is available at https://www.apache.org/licenses/LICENSE-2.0.
*
* This Source Code may also be made available under the following
* Secondary Licenses when the conditions for such availability set
* forth in the Eclipse Public License, v. 2.0 are satisfied: GNU
* General Public License, version 2 with the GNU Classpath
* Exception [1] and GNU General Public License, version 2 with the
* OpenJDK Assembly Exception [2].
*
* [1] https://www.gnu.org/software/classpath/license.html
* [2] http://openjdk.java.net/legal/assembly-exception.html
*
* SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 OR LicenseRef-GPL-2.0 WITH Assembly-exception
*******************************************************************************/


#ifndef VMREGISTER_INCL
#define VMREGISTER_INCL

#include "JitBuilder.hpp"

class VMRegisterMethod : public OMR::JitBuilder::MethodBuilder
{
public:
VMRegisterMethod(OMR::JitBuilder::TypeDictionary *);
virtual bool buildIL();
};

#endif // !defined(VMREGISTER_INCL)

0 comments on commit 02678cf

Please sign in to comment.