Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
18 changes: 12 additions & 6 deletions llvm/include/llvm/CodeGen/Register.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#define LLVM_CODEGEN_REGISTER_H

#include "llvm/MC/MCRegister.h"
#include "llvm/Support/MathExtras.h"
#include <cassert>

namespace llvm {
Expand All @@ -35,19 +36,23 @@ class Register {
// DenseMapInfo<unsigned> uses -1u and -2u.
static_assert(std::numeric_limits<decltype(Reg)>::max() >= 0xFFFFFFFF,
"Reg isn't large enough to hold full range.");
static constexpr unsigned FirstStackSlot = 1u << 30;
static_assert(FirstStackSlot >= MCRegister::LastPhysicalReg);
static constexpr unsigned MaxFrameIndexBitwidth = 30;
static constexpr unsigned StackSlotZero = 1u << MaxFrameIndexBitwidth;
static constexpr const unsigned StackSlotMask = StackSlotZero - 1;
static_assert(StackSlotZero >= MCRegister::LastPhysicalReg);
static constexpr unsigned VirtualRegFlag = 1u << 31;

/// Return true if this is a stack slot.
constexpr bool isStack() const {
return Register::FirstStackSlot <= Reg && Reg < Register::VirtualRegFlag;
return Register::StackSlotZero <= Reg && Reg < Register::VirtualRegFlag;
}

/// Convert a non-negative frame index to a stack slot register value.
static Register index2StackSlot(int FI) {
assert(FI >= 0 && "Cannot hold a negative frame index.");
return Register(FI + Register::FirstStackSlot);
unsigned FIMasked = FI & Register::StackSlotMask;
assert(isUInt<30>(FIMasked) &&
"Frame index must be at most 30 bit as an unsigned integer");
return Register(FIMasked | Register::StackSlotZero);
}

/// Return true if the specified register number is in
Expand Down Expand Up @@ -87,7 +92,8 @@ class Register {
/// Compute the frame index from a register value representing a stack slot.
int stackSlotIndex() const {
assert(isStack() && "Not a stack slot");
return static_cast<int>(Reg - Register::FirstStackSlot);
return static_cast<int>(
SignExtend64<MaxFrameIndexBitwidth>(Reg & Register::StackSlotMask));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use SignExtend32 so you don't need the static_cast?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought int could be smaller then int32_t in theory.

Actually, to me it looks like stackSlotIndex should return int32_t. If int is not 32 bits, the result may not be able to fit, no?

Copy link
Contributor Author

@mgudim mgudim Oct 22, 2025

Choose a reason for hiding this comment

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

@topperc @arsenm

We use unsigned to represent a register, but we implicitly assume that it is 32 bits at least. Isn't it wrong? should we use uint32_t for register values?

EDIT:
Oh, I see there is this static assert:

static_assert(std::numeric_limits<decltype(Reg)>::max() >= 0xFFFFFFFF

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is this OK to merge?

}

constexpr operator unsigned() const { return Reg; }
Expand Down
3 changes: 0 additions & 3 deletions llvm/lib/CodeGen/ReachingDefAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,6 @@ void ReachingDefInfo::processDefs(MachineInstr *MI) {
for (auto &MO : MI->operands()) {
if (MO.isFI()) {
int FrameIndex = MO.getIndex();
assert(FrameIndex >= 0 && "Can't handle negative frame indicies yet!");
if (!isFIDef(*MI, FrameIndex, TII))
continue;
MBBFrameObjsReachingDefs[{MBBNumber, FrameIndex}].push_back(CurInstr);
Expand Down Expand Up @@ -302,8 +301,6 @@ void ReachingDefInfo::print(raw_ostream &OS) {
Register Reg;
if (MO.isFI()) {
int FrameIndex = MO.getIndex();
assert(FrameIndex >= 0 &&
"Can't handle negative frame indicies yet!");
Reg = Register::index2StackSlot(FrameIndex);
} else if (MO.isReg()) {
if (MO.isDef())
Expand Down
1 change: 1 addition & 0 deletions llvm/unittests/CodeGen/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ add_llvm_unittest(CodeGenTests
MachineOperandTest.cpp
MIR2VecTest.cpp
RegAllocScoreTest.cpp
RegisterTest.cpp
PassManagerTest.cpp
ScalableVectorMVTsTest.cpp
SchedBoundary.cpp
Expand Down
33 changes: 33 additions & 0 deletions llvm/unittests/CodeGen/RegisterTest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//===- RegisterTest.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/Register.h"
#include "gtest/gtest.h"

using namespace llvm;

namespace {
TEST(RegisterTest, Idx2StackSlot) {
ASSERT_EQ(Register::index2StackSlot(0), Register::StackSlotZero);
ASSERT_EQ(Register::index2StackSlot(-1),
Register::StackSlotZero | Register::StackSlotMask);
ASSERT_EQ(Register::index2StackSlot(Register::StackSlotMask),
Register::StackSlotZero | Register::StackSlotMask);
ASSERT_EQ(Register::index2StackSlot(1), Register::StackSlotZero | 1);
}

TEST(RegisterTest, StackSlotIndex) {
Register Reg;
std::vector<int64_t> FIs = {0, 1 - 1, (1 << 29) - 1, -(1 << 29)};

for (int64_t FI : FIs) {
Reg = Register::index2StackSlot(FI);
ASSERT_EQ(Reg.stackSlotIndex(), FI);
}
}
} // end namespace
Loading