Skip to content

Commit

Permalink
[OpMap] Fix GCC compiler warnings.
Browse files Browse the repository at this point in the history 
GCC spotted the following warnings:

a) Parenthesis usage around assert in complexSet is not correct in ExampleMain
b) OpMap::m_dialectOps uses the anonymous namespace indirectly
(-Wsubobject-linkage)
  • Loading branch information
@tsymalla-AMD
Thomas Symalla authored and tsymalla-AMD committed Oct 17, 2023
1 parent f9034b8 commit 16a0e93
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 47 deletions.
4 changes: 2 additions & 2 deletions example/ExampleMain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,9 @@ template <bool rpot> const Visitor<VisitorContainer> &getExampleVisitor() {
});
b.addSet(complexSet, [](VisitorNest &self, llvm::Instruction &op) {
assert((op.getOpcode() == Instruction::Ret ||
(isa<IntrinsicInst>)(&op) &&
(isa<IntrinsicInst>(&op) &&
cast<IntrinsicInst>(&op)->getIntrinsicID() ==
Intrinsic::umin) &&
Intrinsic::umin)) &&
"Unexpected operation detected while visiting OpSet!");

if (op.getOpcode() == Instruction::Ret) {
Expand Down
81 changes: 36 additions & 45 deletions include/llvm-dialects/Dialect/OpMap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,35 +37,6 @@
using namespace llvm;
using namespace llvm_dialects;

namespace {

using DialectOpKey = std::pair<StringRef, bool>;

class DialectOpKVUtils {
public:
static DialectOpKey getDialectMapKey(const OpDescription &desc) {
return {desc.getMnemonic(),
desc.getKind() == OpDescription::Kind::DialectWithOverloads};
}
};

template <typename ValueT> struct DialectOpKV final {
DialectOpKey Key;
ValueT Value;

bool operator==(const DialectOpKV &other) const {
return Key.first == other.Key.first && Key.second == other.Key.second &&
Value == other.Value;
}

bool operator==(const OpDescription &desc) const {
const bool isOverload =
desc.getKind() == OpDescription::Kind::DialectWithOverloads;
return Key.first == desc.getMnemonic() && Key.second == isOverload;
}
};
} // namespace

namespace llvm_dialects {

// Forward declarations.
Expand All @@ -87,7 +58,23 @@ template <typename ValueT> class OpMap final {
friend class OpMapIteratorBase<ValueT, true>;
friend class OpMapIteratorBase<ValueT, false>;

using DialectOpKVT = DialectOpKV<ValueT>;
using DialectOpKey = std::pair<StringRef, bool>;

struct DialectOpKV final {
DialectOpKey Key;
ValueT Value;

bool operator==(const DialectOpKV &other) const {
return Key.first == other.Key.first && Key.second == other.Key.second &&
Value == other.Value;
}

bool operator==(const OpDescription &desc) const {
const bool isOverload =
desc.getKind() == OpDescription::Kind::DialectWithOverloads;
return Key.first == desc.getMnemonic() && Key.second == isOverload;
}
};

public:
using iterator = OpMapIteratorBase<ValueT, false>;
Expand Down Expand Up @@ -159,7 +146,7 @@ template <typename ValueT> class OpMap final {
(desc.isIntrinsic() && containsIntrinsic(op));
}

for (const DialectOpKVT &dialectOpKV : m_dialectOps) {
for (const DialectOpKV &dialectOpKV : m_dialectOps) {
if (dialectOpKV == desc)
return true;
}
Expand Down Expand Up @@ -247,7 +234,7 @@ template <typename ValueT> class OpMap final {
// --------------------------------------------------------------------------
template <typename... Ts>
std::pair<iterator, bool> try_emplace(const OpDescription &desc,
Ts &&... vals) {
Ts &&...vals) {
auto found = find(const_cast<OpDescription &>(desc));
if (found)
return {found, false};
Expand All @@ -271,7 +258,7 @@ template <typename ValueT> class OpMap final {

// Find the iterator into the dialect ops.
size_t Idx = 0;
for (DialectOpKVT &dialectOpKV : m_dialectOps) {
for (DialectOpKV &dialectOpKV : m_dialectOps) {
if (dialectOpKV == desc) {
auto it = m_dialectOps.begin();
std::advance(it, Idx);
Expand All @@ -284,9 +271,9 @@ template <typename ValueT> class OpMap final {

// If the entry doesn't exist, construct it and return an iterator to the
// end of dialect ops.
auto it = m_dialectOps.insert(
m_dialectOps.end(),
{DialectOpKVUtils::getDialectMapKey(desc), std::forward<Ts>(vals)...});
auto it =
m_dialectOps.insert(m_dialectOps.end(), {getDialectMapKey(desc),
std::forward<Ts>(vals)...});
return {makeIterator(it, OpDescription::Kind::Dialect, false), true};
}

Expand Down Expand Up @@ -392,16 +379,21 @@ template <typename ValueT> class OpMap final {
private:
DenseMap<unsigned, ValueT> m_coreOpcodes;
DenseMap<unsigned, ValueT> m_intrinsics;
SmallVector<DialectOpKVT, 1> m_dialectOps;
SmallVector<DialectOpKV, 1> m_dialectOps;

template <typename... Args> iterator makeIterator(Args &&... args) {
template <typename... Args> iterator makeIterator(Args &&...args) {
return iterator(this, std::forward<Args>(args)...);
}

template <typename... Args>
const_iterator makeConstIterator(Args &&... args) const {
const_iterator makeConstIterator(Args &&...args) const {
return const_iterator(this, std::forward<Args>(args)...);
}

static DialectOpKey getDialectMapKey(const OpDescription &desc) {
return {desc.getMnemonic(),
desc.getKind() == OpDescription::Kind::DialectWithOverloads};
}
};

/// A simple iterator operating on the internal data structures of the OpMap. It
Expand All @@ -412,19 +404,18 @@ template <typename ValueT> class OpMap final {
/// Note that iterating over an OpMap instance never guarantees the order of
/// insertion.
template <typename ValueT, bool isConst> class OpMapIteratorBase final {
using OpMapT =
std::conditional_t<isConst, const OpMap<ValueT>, OpMap<ValueT>>;
using BaseIteratorT =
std::conditional_t<isConst,
typename DenseMap<unsigned, ValueT>::const_iterator,
typename DenseMap<unsigned, ValueT>::iterator>;
using DialectOpIteratorT = std::conditional_t<
isConst, typename SmallVectorImpl<DialectOpKV<ValueT>>::const_iterator,
typename SmallVectorImpl<DialectOpKV<ValueT>>::iterator>;
isConst, typename SmallVectorImpl<typename OpMapT::DialectOpKV>::const_iterator,
typename SmallVectorImpl<typename OpMapT::DialectOpKV>::iterator>;

using InternalValueT = std::conditional_t<isConst, const ValueT, ValueT>;

using OpMapT =
std::conditional_t<isConst, const OpMap<ValueT>, OpMap<ValueT>>;

friend class OpMap<ValueT>;
friend class OpMapIteratorBase<ValueT, true>;
friend class OpMapIteratorBase<ValueT, false>;
Expand Down Expand Up @@ -723,7 +714,7 @@ template <typename ValueT, bool isConst> class OpMapIteratorBase final {
size_t idx = 0;
bool found = false;
for (auto &dialectOpKV : m_map->m_dialectOps) {
const DialectOpKey &key = dialectOpKV.Key;
const auto &key = dialectOpKV.Key;
if (detail::isOperationDecl(funcName, key.second, key.first)) {
m_desc = {key.second, key.first};
auto it = m_map->m_dialectOps.begin();
Expand Down

0 comments on commit 16a0e93

Please sign in to comment.