Skip to content

Commit

Permalink
Remove C++11-isms that old compilers don't support
Browse files Browse the repository at this point in the history
Older MSVC versions don't support delegating constructors
or alignof(). Use OMR_ALIGNOF() in place of alignof() and
rework the CCData constructors to not require delegation.

Older XL doesn't support std::unique_ptr, manage array
pointers directly.

Also replace nullptr with NULL.

Signed-off-by: Younes Manton <ymanton@ca.ibm.com>
  • Loading branch information
ymanton committed Nov 9, 2020
1 parent c798177 commit 9c7a093
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 84 deletions.
49 changes: 19 additions & 30 deletions compiler/codegen/CCData.cpp
Expand Up @@ -19,6 +19,7 @@
* 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 "omrcomp.h"
#include "OMR/Bytes.hpp"

#include "codegen/CCData.hpp"
Expand All @@ -42,43 +43,31 @@ CCData::key_t CCData::key(const char * const str)
#define DATA_SIZE_FROM_BYTES_SIZE(x) (((x) + (sizeof(data_t)) - 1) / (sizeof(data_t)))

// Converts an alignment in units of bytes to an alignment in units of alignof(data_t).
#define DATA_ALIGNMENT_FROM_BYTES_ALIGNMENT(x) (((x) + (alignof(data_t)) - 1) / (alignof(data_t)))
#define DATA_ALIGNMENT_FROM_BYTES_ALIGNMENT(x) (((x) + (OMR_ALIGNOF(data_t)) - 1) / (OMR_ALIGNOF(data_t)))

CCData::CCData(const size_t sizeBytes)
: _data(new data_t[DATA_SIZE_FROM_BYTES_SIZE(sizeBytes)]), _capacity(DATA_SIZE_FROM_BYTES_SIZE(sizeBytes)), _putIndex(0), _lock(TR::Monitor::create("CCDataMutex")), _releaseData(false)
: _data(new data_t[DATA_SIZE_FROM_BYTES_SIZE(sizeBytes)]), _capacity(DATA_SIZE_FROM_BYTES_SIZE(sizeBytes)), _putIndex(0), _lock(TR::Monitor::create("CCDataMutex")), _releaseData(true)
{
}

CCData::CCData(uint8_t * const storage, const size_t sizeBytes)
: CCData(alignStorage(storage, sizeBytes))
{
}

CCData::CCData(const storage_and_size_pair_t storageAndSize)
: _data(storageAndSize.first), _capacity(storageAndSize.second), _putIndex(0), _lock(TR::Monitor::create("CCDataMutex")), _releaseData(true)
: _putIndex(0), _lock(TR::Monitor::create("CCDataMutex")), _releaseData(false)
{
void *alignedStorage = storage;
size_t sizeBytesAfterAlignment = sizeBytes;
bool success = OMR::align(OMR_ALIGNOF(data_t), sizeof(data_t), alignedStorage, sizeBytesAfterAlignment) != NULL;
TR_ASSERT_FATAL(success, "Can't align CCData storage to required boundary");
_data = reinterpret_cast<data_t *>(alignedStorage);
_capacity = DATA_SIZE_FROM_BYTES_SIZE(sizeBytesAfterAlignment);
}

CCData::~CCData()
{
// Memory for data can either be allocated by this class or passed in via
// the constructor. If allocated, it has to be freed now; if passed in we can't free
// it.
// The _data member var is a smart pointer that will automatically
// free the underlying memory when it goes out of scope. If memory was passed in
// via the constructor we have to release it now to prevent the smart pointer from
// freeing it.
if (_releaseData)
_data.release();
}

const CCData::storage_and_size_pair_t CCData::alignStorage(uint8_t * const storage, size_t sizeBytes)
{
// This function takes buffer, aligns it for data_t, and converts the size in bytes to the size in units of data_t.
// It returns the aligned pointer and adjusted size in an std::pair so that we can use it when calling a constructor in an initializer list.
void *alignedStorage = storage;
OMR::align(alignof(data_t), sizeof(data_t), alignedStorage, sizeBytes);
return std::make_pair(reinterpret_cast<data_t *>(alignedStorage), DATA_SIZE_FROM_BYTES_SIZE(sizeBytes));
delete [] _data;
}

bool CCData::put(const uint8_t * const value, const size_t sizeBytes, const size_t alignmentBytes, const key_t * const key, index_t &index)
Expand All @@ -99,7 +88,7 @@ bool CCData::put(const uint8_t * const value, const size_t sizeBytes, const size
const size_t sizeDataUnits = DATA_SIZE_FROM_BYTES_SIZE(sizeBytes);
const size_t alignmentDataUnits = DATA_ALIGNMENT_FROM_BYTES_ALIGNMENT(alignmentBytes);
const size_t alignmentMask = alignmentDataUnits - 1;
const size_t alignmentPadding = (alignmentDataUnits - ((reinterpret_cast<uintptr_t>(_data.get() + _putIndex) / alignof(data_t)) & alignmentMask)) & alignmentMask;
const size_t alignmentPadding = (alignmentDataUnits - ((reinterpret_cast<uintptr_t>(_data + _putIndex) / OMR_ALIGNOF(data_t)) & alignmentMask)) & alignmentMask;
const size_t remainingCapacity = _capacity - _putIndex;

if (sizeDataUnits + alignmentPadding > remainingCapacity)
Expand All @@ -108,14 +97,14 @@ bool CCData::put(const uint8_t * const value, const size_t sizeBytes, const size
_putIndex += alignmentPadding;
index = _putIndex;

if (key != nullptr)
if (key != NULL)
_mappings[*key] = _putIndex;

if (value != nullptr)
if (value != NULL)
{
std::copy(value,
value + sizeBytes,
reinterpret_cast<uint8_t *>(_data.get() + _putIndex));
reinterpret_cast<uint8_t *>(_data + _putIndex));
}

_putIndex += sizeDataUnits;
Expand All @@ -128,8 +117,8 @@ bool CCData::get(const index_t index, uint8_t * const value, const size_t sizeBy
if (index >= _capacity)
return false;

std::copy(reinterpret_cast<const uint8_t *>(_data.get() + index),
reinterpret_cast<const uint8_t *>(_data.get() + index) + sizeBytes,
std::copy(reinterpret_cast<const uint8_t *>(_data + index),
reinterpret_cast<const uint8_t *>(_data + index) + sizeBytes,
value);

return true;
Expand All @@ -144,9 +133,9 @@ bool CCData::find(const key_t key, index_t * const index) const
bool CCData::find_unsafe(const key_t key, index_t * const index) const
{
auto e = _mappings.find(key);
if (e != _mappings.cend())
if (e != _mappings.end())
{
if (index != nullptr)
if (index != NULL)
*index = e->second;
return true;
}
Expand Down
19 changes: 5 additions & 14 deletions compiler/codegen/CCData.hpp
Expand Up @@ -22,7 +22,6 @@
#ifndef OMR_CCDATA_INCL
#define OMR_CCDATA_INCL

#include <memory>
#include <map>
#include <string>
#include <cstddef>
Expand All @@ -45,9 +44,6 @@ class CCData
/** \typedef data_t Implementation detail. This type represents the units of the table. Typically bytes, but can be some other data type, as long as it can be default-constructed. */
typedef uint8_t data_t;

/** \typedef section_t Implementation detail. This type represents the table itself. It must behave like a pointer to an array of type data_t. */
typedef std::unique_ptr<data_t[]> section_t;

public:
/** \typedef index_t This type represents the indices defined in the public interface of this class. They must behave like integral types. */
typedef size_t index_t;
Expand Down Expand Up @@ -172,7 +168,7 @@ class CCData
* @param[In] T The type of the value to get from the table. The type need not be TriviallyCopyable, since this function doesn't do any copying, but it probably should be for symmetry with the put() functions.
* @param[In] index The index that refers to the value.
* @return A pointer to the value if the index refers to an existing value, nullptr otherwise.
* @return A pointer to the value if the index refers to an existing value, NULL otherwise.
*/
template <typename T>
T* get(const index_t index) const;
Expand All @@ -184,14 +180,9 @@ class CCData
* @param[out] index Optional. A pointer to write the index to. This parameter is ignored unless this function returns true.
* @return True if the given key maps to an index, false otherwise.
*/
bool find(const key_t key, index_t * const index = nullptr) const;
bool find(const key_t key, index_t * const index = NULL) const;

private:
// Helper stuff used by the general (storage, sizeBytes) ctor.
typedef std::pair<data_t * const, const size_t> storage_and_size_pair_t;
CCData(const storage_and_size_pair_t storageAndSize);
static const storage_and_size_pair_t alignStorage(uint8_t * const storage, size_t sizeBytes);

/**
* @brief Checks if the given key maps to an index in this table and returns the index.
* This function is NOT synchronized (hence unsafe).
Expand All @@ -200,11 +191,11 @@ class CCData
* @param[out] index Optional. A pointer to write the index to. This parameter is ignored unless this function returns true.
* @return True if the given key maps to an index, false otherwise.
*/
bool find_unsafe(const key_t key, index_t * const index = nullptr) const;
bool find_unsafe(const key_t key, index_t * const index = NULL) const;

private:
section_t _data; // Could be const were it not for the release() call in the dtor.
const size_t _capacity;
data_t *_data;
size_t _capacity;
size_t _putIndex;
map_t _mappings;
TR::Monitor *_lock;
Expand Down
20 changes: 12 additions & 8 deletions compiler/codegen/CCData_inlines.hpp
Expand Up @@ -40,18 +40,22 @@ CCData::key_t CCData::key(const T value)
template <typename T>
bool CCData::put(const T value, const key_t * const key, index_t &index)
{
#if __cpp_static_assert
static_assert(std::is_trivially_copyable<T>::value == true, "T must be trivially copyable.");
#endif
// std::is_trivially_copyable is a C++11 type trait, but unfortunately there's no test macro for it.
// static_assert is also a C++11 feature, so testing for that would hopefully be enough, but unfortunately some old compilers have static_assert but not is_trivially_copyable,
// hence `#if __cpp_static_assert` is insufficient.
//#if __cpp_static_assert
// static_assert(std::is_trivially_copyable<T>::value == true, "T must be trivially copyable.");
//#endif
return put(reinterpret_cast<const uint8_t *>(&value), sizeof(value), alignof(value), key, index);
}

template <typename T>
bool CCData::get(const index_t index, T &value) const
{
#if __cpp_static_assert
static_assert(std::is_trivially_copyable<T>::value == true, "T must be trivially copyable.");
#endif
// See above.
//#if __cpp_static_assert
// static_assert(std::is_trivially_copyable<T>::value == true, "T must be trivially copyable.");
//#endif
return get(index, reinterpret_cast<uint8_t *>(&value), sizeof(value));
}

Expand All @@ -61,9 +65,9 @@ T* CCData::get(const index_t index) const
// Don't have to check if T is trivially_copyable here since we're not copying to/from a T.
// The caller might, but it is then their responsibility to make sure.
if (index >= _capacity)
return nullptr;
return NULL;

return reinterpret_cast<T *>(_data.get() + index);
return reinterpret_cast<T *>(_data + index);
}

}
Expand Down
62 changes: 31 additions & 31 deletions fvtest/compilertest/tests/CCDataTest.cpp
Expand Up @@ -102,15 +102,15 @@ class GeneratedMethodInfo : public MethodInfo
class TableTest : public OptTestDriver
{
public:
TableTest() : _caseValues(nullptr), _numCaseValues(0) {}
TableTest() : _caseValues(NULL), _numCaseValues(0) {}
void setCaseValues(const int32_t * const caseValues, const size_t numCaseValues)
{
_caseValues = caseValues;
_numCaseValues = numCaseValues;
}
virtual void invokeTests() override
{
EXPECT_NE(_caseValues, nullptr);
EXPECT_TRUE(_caseValues != NULL);
EXPECT_GT(_numCaseValues, 0);
auto compiledMethod = getCompiledMethod<GeneratedMethodInfo::compiled_method_t>();
for (auto i = 0; i < _numCaseValues; ++i)
Expand Down Expand Up @@ -219,9 +219,9 @@ TYPED_TEST(CCDataTest, test_basics_templated)
const TypeParam * const dataPtr = table.get<TypeParam>(index);

// Make sure the data was written.
EXPECT_NE(dataPtr, nullptr);
EXPECT_TRUE(dataPtr != NULL);
// Make sure it was written to an aligned address.
EXPECT_EQ(reinterpret_cast<size_t>(dataPtr) & (alignof(data) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(dataPtr) & (OMR_ALIGNOF(data) - 1), 0);
// Make sure it was written correctly.
EXPECT_EQ(*dataPtr, data);
// We should be able to find something with this key now.
Expand All @@ -244,7 +244,7 @@ TYPED_TEST(CCDataTest, test_basics_templated)
EXPECT_EQ(index, find_index);

// Make sure we can fill the table.
while (table.put(data, nullptr, index));
while (table.put(data, NULL, index));
}

TYPED_TEST(CCDataTest, test_arbitrary_data_templated)
Expand All @@ -261,16 +261,16 @@ TYPED_TEST(CCDataTest, test_arbitrary_data_templated)
CCData::index_t index = INVALID_INDEX;

// Put the data in the table, associate it with the key, retrieve the index.
EXPECT_TRUE(table.put(reinterpret_cast<const uint8_t *>(&data[0]), sizeof(data), alignof(data), &key, index));
EXPECT_TRUE(table.put(reinterpret_cast<const uint8_t *>(&data[0]), sizeof(data), OMR_ALIGNOF(data), &key, index));
// Make sure the index was written.
EXPECT_NE(index, INVALID_INDEX);

const TypeParam * const dataPtr = table.get<TypeParam>(index);

// Make sure the data was written.
EXPECT_NE(dataPtr, nullptr);
EXPECT_TRUE(dataPtr != NULL);
// Make sure it was written to an aligned address.
EXPECT_EQ(reinterpret_cast<size_t>(dataPtr) & (alignof(data) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(dataPtr) & (OMR_ALIGNOF(data) - 1), 0);
// Make sure it was written correctly.
EXPECT_TRUE(std::equal(data, data + sizeof(data)/sizeof(data[0]), dataPtr));
// We should be able to find something with this key now.
Expand All @@ -294,7 +294,7 @@ TYPED_TEST(CCDataTest, test_arbitrary_data_templated)
EXPECT_EQ(index, find_index);

// Make sure we can fill the table.
while (table.put(reinterpret_cast<const uint8_t *>(&data[0]), sizeof(data), alignof(data), nullptr, index));
while (table.put(reinterpret_cast<const uint8_t *>(&data[0]), sizeof(data), OMR_ALIGNOF(data), NULL, index));
}

TYPED_TEST(CCDataTest, test_no_data_reservation_templated)
Expand All @@ -311,18 +311,18 @@ TYPED_TEST(CCDataTest, test_no_data_reservation_templated)
CCData::index_t index2 = INVALID_INDEX;

// Reserve space in the table, associate it with the key, retrieve the index.
EXPECT_TRUE(table.put(nullptr, reservationSize, reservationAlignment, &key, index));
EXPECT_TRUE(table.put(NULL, reservationSize, reservationAlignment, &key, index));
// Make sure the index was written.
EXPECT_NE(index, INVALID_INDEX);
// Try to update the value via the key.
EXPECT_TRUE(table.put(nullptr, reservationSize, reservationAlignment, &key, index2));
EXPECT_TRUE(table.put(NULL, reservationSize, reservationAlignment, &key, index2));
// Make sure the index was written.
EXPECT_EQ(index2, index);

const TypeParam * const dataPtr = table.get<TypeParam>(index);

// Make sure the reservation was done.
EXPECT_NE(dataPtr, nullptr);
EXPECT_TRUE(dataPtr != NULL);
// Make sure we got an aligned address.
EXPECT_EQ(reinterpret_cast<size_t>(dataPtr) & (reservationAlignment - 1), 0);
// We should be able to find something with this key now.
Expand All @@ -336,7 +336,7 @@ TYPED_TEST(CCDataTest, test_no_data_reservation_templated)
EXPECT_EQ(index, find_index);

// Make sure we can fill the table.
while (table.put(nullptr, reservationSize, reservationAlignment, nullptr, index));
while (table.put(NULL, reservationSize, reservationAlignment, NULL, index));
}

#define STRINGIFY(x) #x
Expand Down Expand Up @@ -377,7 +377,7 @@ TYPED_TEST(CCDataTest, test_error_conditions_templated)
CCData::index_t index = INVALID_INDEX;

// Shouldn't be able to put.
EXPECT_FALSE(zeroTable.put(data, nullptr, index));
EXPECT_FALSE(zeroTable.put(data, NULL, index));
// Make sure the index didn't change.
EXPECT_EQ(index, INVALID_INDEX);
}
Expand All @@ -391,7 +391,7 @@ TYPED_TEST(CCDataTest, test_error_conditions_templated)

int count = 0;

while (smallTable.put(data, nullptr, curIndex))
while (smallTable.put(data, NULL, curIndex))
{
// Make sure the index changed.
EXPECT_NE(curIndex, lastIndex);
Expand Down Expand Up @@ -428,7 +428,7 @@ TYPED_TEST(CCDataTest, test_updating_templated)
TypeParam * const dataPtr = table.get<TypeParam>(index1);

// Make sure the data was written.
EXPECT_NE(dataPtr, nullptr);
EXPECT_TRUE(dataPtr != NULL);
// Make sure it wasn't updated.
EXPECT_EQ(*dataPtr, data1);
// Change the value via a pointer.
Expand Down Expand Up @@ -543,21 +543,21 @@ TEST(AllTypesCCDataTest, test_basics)
const double * const ptr_doubleA = table.get<const double>(doubleAIndex);
const double * const ptr_doubleB = table.get<const double>(doubleBIndex);

EXPECT_EQ(reinterpret_cast<size_t>(ptr_classAptr) & (alignof(*ptr_classAptr) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_classBptr) & (alignof(*ptr_classBptr) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_funcAptr) & (alignof(*ptr_funcAptr) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_funcBptr) & (alignof(*ptr_funcBptr) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_intA) & (alignof(*ptr_intA) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_intB) & (alignof(*ptr_intB) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_shortA) & (alignof(*ptr_shortA) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_shortB) & (alignof(*ptr_shortB) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_floatA) & (alignof(*ptr_floatA) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_floatB) & (alignof(*ptr_floatB) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_doubleA) & (alignof(*ptr_doubleA) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_doubleB) & (alignof(*ptr_doubleB) - 1), 0);

void *out_classAptr = nullptr, *out_classBptr = nullptr;
void *out_funcAptr = nullptr, *out_funcBptr = nullptr;
EXPECT_EQ(reinterpret_cast<size_t>(ptr_classAptr) & (OMR_ALIGNOF(*ptr_classAptr) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_classBptr) & (OMR_ALIGNOF(*ptr_classBptr) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_funcAptr) & (OMR_ALIGNOF(*ptr_funcAptr) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_funcBptr) & (OMR_ALIGNOF(*ptr_funcBptr) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_intA) & (OMR_ALIGNOF(*ptr_intA) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_intB) & (OMR_ALIGNOF(*ptr_intB) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_shortA) & (OMR_ALIGNOF(*ptr_shortA) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_shortB) & (OMR_ALIGNOF(*ptr_shortB) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_floatA) & (OMR_ALIGNOF(*ptr_floatA) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_floatB) & (OMR_ALIGNOF(*ptr_floatB) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_doubleA) & (OMR_ALIGNOF(*ptr_doubleA) - 1), 0);
EXPECT_EQ(reinterpret_cast<size_t>(ptr_doubleB) & (OMR_ALIGNOF(*ptr_doubleB) - 1), 0);

void *out_classAptr = NULL, *out_classBptr = NULL;
void *out_funcAptr = NULL, *out_funcBptr = NULL;
int out_intA = ~intA, out_intB = ~intB;
short out_shortA = ~shortA, out_shortB = ~shortB;
float out_floatA = -floatA, out_floatB = -floatB;
Expand Down
3 changes: 2 additions & 1 deletion fvtest/coretest/TestBytes.cpp
Expand Up @@ -19,6 +19,7 @@
* 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 <omrcomp.h>
#include <OMR/Bytes.hpp>
#include <gtest/gtest.h>

Expand Down Expand Up @@ -187,7 +188,7 @@ TEST(TestBytes, AlignMaximumSizeFor16byteAlignment)
TEST(TestBytes, AlignPointers)
{
const size_t size = sizeof(void*);
const size_t alignment = alignof(void*);
const size_t alignment = OMR_ALIGNOF(void*);
const size_t totalSpace = size * 3 - 1;
char buffer[totalSpace] = {0};
void * const lastValidAddress = &buffer[totalSpace - size];
Expand Down

0 comments on commit 9c7a093

Please sign in to comment.