Skip to content

[GlobalISel] Optimize ULEB128 usage #90565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ enum {
/// operand.
/// - InsnID(ULEB128) - Instruction ID
/// - MMOIdx(ULEB128) - MMO index
/// - NumAddrSpace(ULEB128) - Number of valid address spaces
/// - NumAddrSpace(1) - Number of valid address spaces
/// - AddrSpaceN(ULEB128) - An allowed space of the memory access
/// - AddrSpaceN+1 ...
GIM_CheckMemoryAddressSpace,
Expand All @@ -177,7 +177,7 @@ enum {
/// memory operand.
/// - InsnID(ULEB128) - Instruction ID
/// - MMOIdx(ULEB128) - MMO index
/// - MinAlign(ULEB128) - Minimum acceptable alignment
/// - MinAlign(1) - Minimum acceptable alignment
GIM_CheckMemoryAlignment,

/// Check the size of the memory access for the given machine memory operand
Expand Down Expand Up @@ -713,6 +713,28 @@ class GIMatchTableExecutor {
memcpy(&Ret, MatchTable, sizeof(Ret));
return Ret;
}

public:
// Faster ULEB128 decoder tailored for the Match Table Executor.
//
// - Arguments are fixed to avoid mid-function checks.
// - Unchecked execution, assumes no error.
// - Fast common case handling (1 byte values).
LLVM_ATTRIBUTE_ALWAYS_INLINE static uint64_t
fastDecodeULEB128(const uint8_t *LLVM_ATTRIBUTE_RESTRICT MatchTable,
uint64_t &CurrentIdx) {
uint64_t Value = MatchTable[CurrentIdx++];
if (LLVM_UNLIKELY(Value >= 128)) {
Value &= 0x7f;
unsigned Shift = 7;
do {
uint64_t Slice = MatchTable[CurrentIdx] & 0x7f;
Value += Slice << Shift;
Shift += 7;
} while (MatchTable[CurrentIdx++] >= 128);
}
return Value;
}
};

} // end namespace llvm
Expand Down
15 changes: 6 additions & 9 deletions llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,7 @@ bool GIMatchTableExecutor::executeMatchTable(
};

const auto readULEB = [&]() {
unsigned N = 0;
uint64_t Val = decodeULEB128(MatchTable + CurrentIdx, &N);
CurrentIdx += N;
return Val;
return fastDecodeULEB128(MatchTable, CurrentIdx);
};

// Convenience function to return a signed value. This avoids
Expand Down Expand Up @@ -476,7 +473,7 @@ bool GIMatchTableExecutor::executeMatchTable(
}
case GIM_CheckAtomicOrdering: {
uint64_t InsnID = readULEB();
auto Ordering = (AtomicOrdering)readULEB();
auto Ordering = (AtomicOrdering)MatchTable[CurrentIdx++];
DEBUG_WITH_TYPE(TgtExecutor::getName(),
dbgs() << CurrentIdx << ": GIM_CheckAtomicOrdering(MIs["
<< InsnID << "], " << (uint64_t)Ordering << ")\n");
Expand All @@ -493,7 +490,7 @@ bool GIMatchTableExecutor::executeMatchTable(
}
case GIM_CheckAtomicOrderingOrStrongerThan: {
uint64_t InsnID = readULEB();
auto Ordering = (AtomicOrdering)readULEB();
auto Ordering = (AtomicOrdering)MatchTable[CurrentIdx++];
DEBUG_WITH_TYPE(TgtExecutor::getName(),
dbgs() << CurrentIdx
<< ": GIM_CheckAtomicOrderingOrStrongerThan(MIs["
Expand All @@ -511,7 +508,7 @@ bool GIMatchTableExecutor::executeMatchTable(
}
case GIM_CheckAtomicOrderingWeakerThan: {
uint64_t InsnID = readULEB();
auto Ordering = (AtomicOrdering)readULEB();
auto Ordering = (AtomicOrdering)MatchTable[CurrentIdx++];
DEBUG_WITH_TYPE(TgtExecutor::getName(),
dbgs() << CurrentIdx
<< ": GIM_CheckAtomicOrderingWeakerThan(MIs["
Expand All @@ -531,7 +528,7 @@ bool GIMatchTableExecutor::executeMatchTable(
uint64_t InsnID = readULEB();
uint64_t MMOIdx = readULEB();
// This accepts a list of possible address spaces.
const uint64_t NumAddrSpace = readULEB();
const uint64_t NumAddrSpace = MatchTable[CurrentIdx++];

if (State.MIs[InsnID]->getNumMemOperands() <= MMOIdx) {
if (handleReject() == RejectAndGiveUp)
Expand Down Expand Up @@ -568,7 +565,7 @@ bool GIMatchTableExecutor::executeMatchTable(
case GIM_CheckMemoryAlignment: {
uint64_t InsnID = readULEB();
uint64_t MMOIdx = readULEB();
uint64_t MinAlign = readULEB();
uint64_t MinAlign = MatchTable[CurrentIdx++];

assert(State.MIs[InsnID] != nullptr && "Used insn before defined");

Expand Down
8 changes: 8 additions & 0 deletions llvm/include/llvm/Support/Compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,14 @@
#define LLVM_ATTRIBUTE_RETURNS_NONNULL
#endif

/// LLVM_ATTRIBUTE_RESTRICT - Annotates a pointer to tell the compiler that
/// it is not aliased in the current scope.
#if defined(__clang__) || defined(__GNUC__) || defined(_MSC_VER)
#define LLVM_ATTRIBUTE_RESTRICT __restrict
#else
#define LLVM_ATTRIBUTE_RESTRICT
#endif

/// \macro LLVM_ATTRIBUTE_RETURNS_NOALIAS Used to mark a function as returning a
/// pointer that does not alias any other valid pointer.
#ifdef __GNUC__
Expand Down
1 change: 1 addition & 0 deletions llvm/unittests/CodeGen/GlobalISel/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ set(LLVM_LINK_COMPONENTS
add_llvm_unittest(GlobalISelTests
ConstantFoldingTest.cpp
CSETest.cpp
GIMatchTableExecutorTest.cpp
LegalizerTest.cpp
LegalizerHelperTest.cpp
LegalizerInfoTest.cpp
Expand Down
49 changes: 49 additions & 0 deletions llvm/unittests/CodeGen/GlobalISel/GIMatchTableExecutorTest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
//===- GIMatchTableExecutorTest.cpp ---------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h"
#include "gtest/gtest.h"

using namespace llvm;

TEST(GlobalISelLEB128Test, fastDecodeULEB128) {
#define EXPECT_DECODE_ULEB128_EQ(EXPECTED, VALUE) \
do { \
uint64_t ActualSize = 0; \
uint64_t Actual = GIMatchTableExecutor::fastDecodeULEB128( \
reinterpret_cast<const uint8_t *>(VALUE), ActualSize); \
EXPECT_EQ(sizeof(VALUE) - 1, ActualSize); \
EXPECT_EQ(EXPECTED, Actual); \
} while (0)

EXPECT_DECODE_ULEB128_EQ(0u, "\x00");
EXPECT_DECODE_ULEB128_EQ(1u, "\x01");
EXPECT_DECODE_ULEB128_EQ(63u, "\x3f");
EXPECT_DECODE_ULEB128_EQ(64u, "\x40");
EXPECT_DECODE_ULEB128_EQ(0x7fu, "\x7f");
EXPECT_DECODE_ULEB128_EQ(0x80u, "\x80\x01");
EXPECT_DECODE_ULEB128_EQ(0x81u, "\x81\x01");
EXPECT_DECODE_ULEB128_EQ(0x90u, "\x90\x01");
EXPECT_DECODE_ULEB128_EQ(0xffu, "\xff\x01");
EXPECT_DECODE_ULEB128_EQ(0x100u, "\x80\x02");
EXPECT_DECODE_ULEB128_EQ(0x101u, "\x81\x02");
EXPECT_DECODE_ULEB128_EQ(4294975616ULL, "\x80\xc1\x80\x80\x10");

// Decode ULEB128 with extra padding bytes
EXPECT_DECODE_ULEB128_EQ(0u, "\x80\x00");
EXPECT_DECODE_ULEB128_EQ(0u, "\x80\x80\x00");
EXPECT_DECODE_ULEB128_EQ(0x7fu, "\xff\x00");
EXPECT_DECODE_ULEB128_EQ(0x7fu, "\xff\x80\x00");
EXPECT_DECODE_ULEB128_EQ(0x80u, "\x80\x81\x00");
EXPECT_DECODE_ULEB128_EQ(0x80u, "\x80\x81\x80\x00");
EXPECT_DECODE_ULEB128_EQ(0x80u, "\x80\x81\x80\x80\x80\x80\x80\x80\x80\x00");
EXPECT_DECODE_ULEB128_EQ(0x80000000'00000000ul,
"\x80\x80\x80\x80\x80\x80\x80\x80\x80\x01");

#undef EXPECT_DECODE_ULEB128_EQ
}
Original file line number Diff line number Diff line change
Expand Up @@ -1565,13 +1565,14 @@ bool MemoryAddressSpacePredicateMatcher::isIdentical(

void MemoryAddressSpacePredicateMatcher::emitPredicateOpcodes(
MatchTable &Table, RuleMatcher &Rule) const {
assert(AddrSpaces.size() < 256);
Table << MatchTable::Opcode("GIM_CheckMemoryAddressSpace")
<< MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID)
<< MatchTable::Comment("MMO")
<< MatchTable::ULEB128Value(MMOIdx)
// Encode number of address spaces to expect.
<< MatchTable::Comment("NumAddrSpace")
<< MatchTable::ULEB128Value(AddrSpaces.size());
<< MatchTable::IntValue(1, AddrSpaces.size());
for (unsigned AS : AddrSpaces)
Table << MatchTable::Comment("AddrSpace") << MatchTable::ULEB128Value(AS);

Expand All @@ -1590,10 +1591,13 @@ bool MemoryAlignmentPredicateMatcher::isIdentical(

void MemoryAlignmentPredicateMatcher::emitPredicateOpcodes(
MatchTable &Table, RuleMatcher &Rule) const {
// TODO: we could support more, just need to emit the right opcode or switch
// to log alignment.
assert(MinAlign < 256);
Table << MatchTable::Opcode("GIM_CheckMemoryAlignment")
<< MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID)
<< MatchTable::Comment("MMO") << MatchTable::ULEB128Value(MMOIdx)
<< MatchTable::Comment("MinAlign") << MatchTable::ULEB128Value(MinAlign)
<< MatchTable::Comment("MinAlign") << MatchTable::IntValue(1, MinAlign)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe assert MinAlign < 256 somewhere? This is not log alignment, so it would break if some vector load instruction actually required 256-byte or greater alignment.

<< MatchTable::LineBreak;
}

Expand Down