https://github.com/MacDue updated https://github.com/llvm/llvm-project/pull/158120
>From 6f14b6da1355898c070e4bf836ca05974d65b5a7 Mon Sep 17 00:00:00 2001 From: Benjamin Maxwell <benjamin.maxw...@arm.com> Date: Thu, 11 Sep 2025 14:51:51 +0000 Subject: [PATCH 1/5] [ADT] Add and use (for AArch64) `ValueWithSentinel<T, Sentinel>` This intends to add a slightly safer std::optional-like interface over checking for sentinel values. This is used a few times on AArch64, but it looks like it could apply to other places/targets too. --- llvm/include/llvm/ADT/ValueWithSentinel.h | 75 +++++++++++++++++++ .../Target/AArch64/AArch64FrameLowering.cpp | 31 ++++---- .../Target/AArch64/AArch64ISelLowering.cpp | 8 +- .../AArch64/AArch64MachineFunctionInfo.h | 31 ++++---- llvm/unittests/ADT/CMakeLists.txt | 1 + llvm/unittests/ADT/ValueWithSentinelTest.cpp | 37 +++++++++ 6 files changed, 149 insertions(+), 34 deletions(-) create mode 100644 llvm/include/llvm/ADT/ValueWithSentinel.h create mode 100644 llvm/unittests/ADT/ValueWithSentinelTest.cpp diff --git a/llvm/include/llvm/ADT/ValueWithSentinel.h b/llvm/include/llvm/ADT/ValueWithSentinel.h new file mode 100644 index 0000000000000..011d03dcdefc9 --- /dev/null +++ b/llvm/include/llvm/ADT/ValueWithSentinel.h @@ -0,0 +1,75 @@ +//===----------------------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file defines the ValueWithSentinel class, which is a type akin to a +/// std::optional, but uses a sentinel rather than an additional "valid" flag. +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_ADT_VALUEWITHSENTINEL_H +#define LLVM_ADT_VALUEWITHSENTINEL_H + +#include <cassert> +#include <limits> +#include <utility> + +namespace llvm { + +template <typename T, T Sentinel> class ValueWithSentinel { +public: + ValueWithSentinel() = default; + + ValueWithSentinel(T Value) : Value(std::move(Value)) { + assert(Value != Sentinel && "Value is sentinel (use default constructor)"); + }; + + ValueWithSentinel &operator=(T const &NewValue) { + assert(NewValue != Sentinel && "Assigned to sentinel (use .clear())"); + Value = NewValue; + return *this; + } + + bool operator==(ValueWithSentinel const &Other) const { + return Value == Other.Value; + } + + bool operator!=(ValueWithSentinel const &Other) const { + return !(*this == Other); + } + + T &operator*() { + assert(has_value() && "Invalid value"); + return Value; + } + const T &operator*() const { + return const_cast<ValueWithSentinel &>(*this).operator*(); + } + + T *operator->() { return &operator*(); } + T const *operator->() const { return &operator*(); } + + T &value() { return operator*(); } + T const &value() const { return operator*(); } + + bool has_value() const { return Value != Sentinel; } + explicit operator bool() const { return has_value(); } + + void clear() { Value = Sentinel; } + +private: + T Value{Sentinel}; +}; + +template <typename T> +using ValueWithSentinelNumericMax = + ValueWithSentinel<T, std::numeric_limits<T>::max()>; + +} // namespace llvm + +#endif diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp index 175b5e04d82ff..45a56c409ab9c 100644 --- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp +++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp @@ -3479,7 +3479,7 @@ bool AArch64FrameLowering::assignCalleeSavedSpillSlots( } Register LastReg = 0; - int HazardSlotIndex = std::numeric_limits<int>::max(); + ValueWithSentinelNumericMax<int> HazardSlotIndex; for (auto &CS : CSI) { MCRegister Reg = CS.getReg(); const TargetRegisterClass *RC = RegInfo->getMinimalPhysRegClass(Reg); @@ -3488,16 +3488,16 @@ bool AArch64FrameLowering::assignCalleeSavedSpillSlots( if (AFI->hasStackHazardSlotIndex() && (!LastReg || !AArch64InstrInfo::isFpOrNEON(LastReg)) && AArch64InstrInfo::isFpOrNEON(Reg)) { - assert(HazardSlotIndex == std::numeric_limits<int>::max() && + assert(!HazardSlotIndex.has_value() && "Unexpected register order for hazard slot"); HazardSlotIndex = MFI.CreateStackObject(StackHazardSize, Align(8), true); - LLVM_DEBUG(dbgs() << "Created CSR Hazard at slot " << HazardSlotIndex + LLVM_DEBUG(dbgs() << "Created CSR Hazard at slot " << *HazardSlotIndex << "\n"); - AFI->setStackHazardCSRSlotIndex(HazardSlotIndex); - if ((unsigned)HazardSlotIndex < MinCSFrameIndex) - MinCSFrameIndex = HazardSlotIndex; - if ((unsigned)HazardSlotIndex > MaxCSFrameIndex) - MaxCSFrameIndex = HazardSlotIndex; + AFI->setStackHazardCSRSlotIndex(*HazardSlotIndex); + if ((unsigned)*HazardSlotIndex < MinCSFrameIndex) + MinCSFrameIndex = *HazardSlotIndex; + if ((unsigned)*HazardSlotIndex > MaxCSFrameIndex) + MaxCSFrameIndex = *HazardSlotIndex; } unsigned Size = RegInfo->getSpillSize(*RC); @@ -3524,16 +3524,15 @@ bool AArch64FrameLowering::assignCalleeSavedSpillSlots( } // Add hazard slot in the case where no FPR CSRs are present. - if (AFI->hasStackHazardSlotIndex() && - HazardSlotIndex == std::numeric_limits<int>::max()) { + if (AFI->hasStackHazardSlotIndex() && !HazardSlotIndex.has_value()) { HazardSlotIndex = MFI.CreateStackObject(StackHazardSize, Align(8), true); - LLVM_DEBUG(dbgs() << "Created CSR Hazard at slot " << HazardSlotIndex + LLVM_DEBUG(dbgs() << "Created CSR Hazard at slot " << *HazardSlotIndex << "\n"); - AFI->setStackHazardCSRSlotIndex(HazardSlotIndex); - if ((unsigned)HazardSlotIndex < MinCSFrameIndex) - MinCSFrameIndex = HazardSlotIndex; - if ((unsigned)HazardSlotIndex > MaxCSFrameIndex) - MaxCSFrameIndex = HazardSlotIndex; + AFI->setStackHazardCSRSlotIndex(*HazardSlotIndex); + if ((unsigned)*HazardSlotIndex < MinCSFrameIndex) + MinCSFrameIndex = *HazardSlotIndex; + if ((unsigned)*HazardSlotIndex > MaxCSFrameIndex) + MaxCSFrameIndex = *HazardSlotIndex; } return true; diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp index 5ffaf2c49b4c0..9153c470004df 100644 --- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp +++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp @@ -3061,10 +3061,10 @@ AArch64TargetLowering::EmitInitTPIDR2Object(MachineInstr &MI, BuildMI(*BB, MI, MI.getDebugLoc(), TII->get(AArch64::STPXi)) .addReg(MI.getOperand(0).getReg()) .addReg(MI.getOperand(1).getReg()) - .addFrameIndex(TPIDR2.FrameIndex) + .addFrameIndex(*TPIDR2.FrameIndex) .addImm(0); } else - MFI.RemoveStackObject(TPIDR2.FrameIndex); + MFI.RemoveStackObject(*TPIDR2.FrameIndex); BB->remove_instr(&MI); return BB; @@ -9399,7 +9399,7 @@ AArch64TargetLowering::LowerCall(CallLoweringInfo &CLI, if (RequiresLazySave) { TPIDR2Object &TPIDR2 = FuncInfo->getTPIDR2Obj(); SDValue TPIDR2ObjAddr = DAG.getFrameIndex( - TPIDR2.FrameIndex, + *TPIDR2.FrameIndex, DAG.getTargetLoweringInfo().getFrameIndexTy(DAG.getDataLayout())); Chain = DAG.getNode( ISD::INTRINSIC_VOID, DL, MVT::Other, Chain, @@ -9956,7 +9956,7 @@ AArch64TargetLowering::LowerCall(CallLoweringInfo &CLI, // RESTORE_ZA pseudo. SDValue Glue; SDValue TPIDR2Block = DAG.getFrameIndex( - TPIDR2.FrameIndex, + *TPIDR2.FrameIndex, DAG.getTargetLoweringInfo().getFrameIndexTy(DAG.getDataLayout())); Result = DAG.getCopyToReg(Result, DL, AArch64::X0, TPIDR2Block, Glue); Result = diff --git a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h index 993cff112ba84..def796aaaface 100644 --- a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h +++ b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h @@ -18,6 +18,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/ValueWithSentinel.h" #include "llvm/CodeGen/CallingConvLower.h" #include "llvm/CodeGen/MIRYamlMapping.h" #include "llvm/CodeGen/MachineFrameInfo.h" @@ -38,7 +39,7 @@ class AArch64Subtarget; class MachineInstr; struct TPIDR2Object { - int FrameIndex = std::numeric_limits<int>::max(); + ValueWithSentinelNumericMax<int> FrameIndex; unsigned Uses = 0; }; @@ -114,8 +115,8 @@ class AArch64FunctionInfo final : public MachineFunctionInfo { /// The stack slots used to add space between FPR and GPR accesses when using /// hazard padding. StackHazardCSRSlotIndex is added between GPR and FPR CSRs. /// StackHazardSlotIndex is added between (sorted) stack objects. - int StackHazardSlotIndex = std::numeric_limits<int>::max(); - int StackHazardCSRSlotIndex = std::numeric_limits<int>::max(); + ValueWithSentinelNumericMax<int> StackHazardSlotIndex; + ValueWithSentinelNumericMax<int> StackHazardCSRSlotIndex; /// True if this function has a subset of CSRs that is handled explicitly via /// copies. @@ -205,7 +206,7 @@ class AArch64FunctionInfo final : public MachineFunctionInfo { bool HasSwiftAsyncContext = false; /// The stack slot where the Swift asynchronous context is stored. - int SwiftAsyncContextFrameIdx = std::numeric_limits<int>::max(); + ValueWithSentinelNumericMax<int> SwiftAsyncContextFrameIdx; bool IsMTETagged = false; @@ -372,16 +373,16 @@ class AArch64FunctionInfo final : public MachineFunctionInfo { MaxOffset = std::max<int64_t>(Offset + ObjSize, MaxOffset); } - if (SwiftAsyncContextFrameIdx != std::numeric_limits<int>::max()) { + if (SwiftAsyncContextFrameIdx.has_value()) { int64_t Offset = MFI.getObjectOffset(getSwiftAsyncContextFrameIdx()); int64_t ObjSize = MFI.getObjectSize(getSwiftAsyncContextFrameIdx()); MinOffset = std::min<int64_t>(Offset, MinOffset); MaxOffset = std::max<int64_t>(Offset + ObjSize, MaxOffset); } - if (StackHazardCSRSlotIndex != std::numeric_limits<int>::max()) { - int64_t Offset = MFI.getObjectOffset(StackHazardCSRSlotIndex); - int64_t ObjSize = MFI.getObjectSize(StackHazardCSRSlotIndex); + if (StackHazardCSRSlotIndex.has_value()) { + int64_t Offset = MFI.getObjectOffset(*StackHazardCSRSlotIndex); + int64_t ObjSize = MFI.getObjectSize(*StackHazardCSRSlotIndex); MinOffset = std::min<int64_t>(Offset, MinOffset); MaxOffset = std::max<int64_t>(Offset + ObjSize, MaxOffset); } @@ -447,16 +448,16 @@ class AArch64FunctionInfo final : public MachineFunctionInfo { void setVarArgsFPRSize(unsigned Size) { VarArgsFPRSize = Size; } bool hasStackHazardSlotIndex() const { - return StackHazardSlotIndex != std::numeric_limits<int>::max(); + return StackHazardSlotIndex.has_value(); } - int getStackHazardSlotIndex() const { return StackHazardSlotIndex; } + int getStackHazardSlotIndex() const { return *StackHazardSlotIndex; } void setStackHazardSlotIndex(int Index) { - assert(StackHazardSlotIndex == std::numeric_limits<int>::max()); + assert(!StackHazardSlotIndex.has_value()); StackHazardSlotIndex = Index; } - int getStackHazardCSRSlotIndex() const { return StackHazardCSRSlotIndex; } + int getStackHazardCSRSlotIndex() const { return *StackHazardCSRSlotIndex; } void setStackHazardCSRSlotIndex(int Index) { - assert(StackHazardCSRSlotIndex == std::numeric_limits<int>::max()); + assert(!StackHazardCSRSlotIndex.has_value()); StackHazardCSRSlotIndex = Index; } @@ -574,7 +575,9 @@ class AArch64FunctionInfo final : public MachineFunctionInfo { void setSwiftAsyncContextFrameIdx(int FI) { SwiftAsyncContextFrameIdx = FI; } - int getSwiftAsyncContextFrameIdx() const { return SwiftAsyncContextFrameIdx; } + int getSwiftAsyncContextFrameIdx() const { + return *SwiftAsyncContextFrameIdx; + } bool needsDwarfUnwindInfo(const MachineFunction &MF) const; bool needsAsyncDwarfUnwindInfo(const MachineFunction &MF) const; diff --git a/llvm/unittests/ADT/CMakeLists.txt b/llvm/unittests/ADT/CMakeLists.txt index dafd73518aedb..ef46ea2f8a083 100644 --- a/llvm/unittests/ADT/CMakeLists.txt +++ b/llvm/unittests/ADT/CMakeLists.txt @@ -93,6 +93,7 @@ add_llvm_unittest(ADTTests TwineTest.cpp TypeSwitchTest.cpp TypeTraitsTest.cpp + ValueWithSentinelTest.cpp ) target_link_libraries(ADTTests PRIVATE LLVMTestingSupport) diff --git a/llvm/unittests/ADT/ValueWithSentinelTest.cpp b/llvm/unittests/ADT/ValueWithSentinelTest.cpp new file mode 100644 index 0000000000000..b02434c73707c --- /dev/null +++ b/llvm/unittests/ADT/ValueWithSentinelTest.cpp @@ -0,0 +1,37 @@ +//===----------------------------------------------------------------------===// +// +// 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/ADT/ValueWithSentinel.h" +#include "gtest/gtest.h" + +using namespace llvm; + +namespace { + +TEST(ValueWithSentinelTest, Basic) { + ValueWithSentinelNumericMax<int> Value; + EXPECT_FALSE(Value.has_value()); + + Value = 1000; + EXPECT_TRUE(Value.has_value()); + + EXPECT_EQ(Value, 1000); + EXPECT_EQ(Value.value(), 1000); + + Value.clear(); + EXPECT_FALSE(Value.has_value()); + + ValueWithSentinelNumericMax<int> OtherValue(99); + EXPECT_TRUE(OtherValue.has_value()); + EXPECT_NE(Value, OtherValue); + + Value = OtherValue; + EXPECT_EQ(Value, OtherValue); +} + +} // end anonymous namespace >From 13c85fd3a3f51a4534fc2b75680b1110b7bcd1ea Mon Sep 17 00:00:00 2001 From: Benjamin Maxwell <benjamin.maxw...@arm.com> Date: Thu, 11 Sep 2025 18:54:00 +0000 Subject: [PATCH 2/5] Fixups --- llvm/include/llvm/ADT/ValueWithSentinel.h | 25 +++++++++---------- .../Target/AArch64/AArch64FrameLowering.cpp | 8 +++--- llvm/unittests/ADT/ValueWithSentinelTest.cpp | 25 +++++++++++++++++++ 3 files changed, 41 insertions(+), 17 deletions(-) diff --git a/llvm/include/llvm/ADT/ValueWithSentinel.h b/llvm/include/llvm/ADT/ValueWithSentinel.h index 011d03dcdefc9..4c45258724459 100644 --- a/llvm/include/llvm/ADT/ValueWithSentinel.h +++ b/llvm/include/llvm/ADT/ValueWithSentinel.h @@ -29,36 +29,35 @@ template <typename T, T Sentinel> class ValueWithSentinel { assert(Value != Sentinel && "Value is sentinel (use default constructor)"); }; - ValueWithSentinel &operator=(T const &NewValue) { - assert(NewValue != Sentinel && "Assigned to sentinel (use .clear())"); + ValueWithSentinel &operator=(const T &NewValue) { + assert(NewValue != Sentinel && "NewValue is sentinel (use .clear())"); Value = NewValue; return *this; } - bool operator==(ValueWithSentinel const &Other) const { + bool operator==(const ValueWithSentinel &Other) const { return Value == Other.Value; } - bool operator!=(ValueWithSentinel const &Other) const { + bool operator!=(const ValueWithSentinel &Other) const { return !(*this == Other); } - T &operator*() { - assert(has_value() && "Invalid value"); + T &value() { + assert(has_value() && ".value() called on sentinel"); return Value; } - const T &operator*() const { - return const_cast<ValueWithSentinel &>(*this).operator*(); + const T &value() const { + return const_cast<ValueWithSentinel &>(*this).value(); } - T *operator->() { return &operator*(); } - T const *operator->() const { return &operator*(); } - - T &value() { return operator*(); } - T const &value() const { return operator*(); } + T &operator*() { return value(); } + const T &operator*() const { return value(); } bool has_value() const { return Value != Sentinel; } + explicit operator bool() const { return has_value(); } + explicit operator T() const { return value(); } void clear() { Value = Sentinel; } diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp index 45a56c409ab9c..2dcbd1b007b33 100644 --- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp +++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp @@ -3494,9 +3494,9 @@ bool AArch64FrameLowering::assignCalleeSavedSpillSlots( LLVM_DEBUG(dbgs() << "Created CSR Hazard at slot " << *HazardSlotIndex << "\n"); AFI->setStackHazardCSRSlotIndex(*HazardSlotIndex); - if ((unsigned)*HazardSlotIndex < MinCSFrameIndex) + if (static_cast<unsigned>(*HazardSlotIndex) < MinCSFrameIndex) MinCSFrameIndex = *HazardSlotIndex; - if ((unsigned)*HazardSlotIndex > MaxCSFrameIndex) + if (static_cast<unsigned>(*HazardSlotIndex) > MaxCSFrameIndex) MaxCSFrameIndex = *HazardSlotIndex; } @@ -3529,9 +3529,9 @@ bool AArch64FrameLowering::assignCalleeSavedSpillSlots( LLVM_DEBUG(dbgs() << "Created CSR Hazard at slot " << *HazardSlotIndex << "\n"); AFI->setStackHazardCSRSlotIndex(*HazardSlotIndex); - if ((unsigned)*HazardSlotIndex < MinCSFrameIndex) + if (static_cast<unsigned>(*HazardSlotIndex) < MinCSFrameIndex) MinCSFrameIndex = *HazardSlotIndex; - if ((unsigned)*HazardSlotIndex > MaxCSFrameIndex) + if (static_cast<unsigned>(*HazardSlotIndex) > MaxCSFrameIndex) MaxCSFrameIndex = *HazardSlotIndex; } diff --git a/llvm/unittests/ADT/ValueWithSentinelTest.cpp b/llvm/unittests/ADT/ValueWithSentinelTest.cpp index b02434c73707c..a972161450a62 100644 --- a/llvm/unittests/ADT/ValueWithSentinelTest.cpp +++ b/llvm/unittests/ADT/ValueWithSentinelTest.cpp @@ -14,24 +14,49 @@ using namespace llvm; namespace { TEST(ValueWithSentinelTest, Basic) { + // Default constructor should equal sentinel. ValueWithSentinelNumericMax<int> Value; EXPECT_FALSE(Value.has_value()); + EXPECT_FALSE(bool(Value)); + // Assignment operator. Value = 1000; EXPECT_TRUE(Value.has_value()); + // .value(), operator*, implicit constructor, explicit conversion EXPECT_EQ(Value, 1000); EXPECT_EQ(Value.value(), 1000); + EXPECT_EQ(*Value, 1000); + EXPECT_EQ(int(Value), 1000); + // .clear() should set value to sentinel Value.clear(); EXPECT_FALSE(Value.has_value()); + EXPECT_FALSE(bool(Value)); + // construction from value, comparison operators ValueWithSentinelNumericMax<int> OtherValue(99); EXPECT_TRUE(OtherValue.has_value()); + EXPECT_TRUE(bool(OtherValue)); + EXPECT_EQ(OtherValue, 99); EXPECT_NE(Value, OtherValue); Value = OtherValue; EXPECT_EQ(Value, OtherValue); } +TEST(ValueWithSentinelTest, PointerType) { + ValueWithSentinel<int *, nullptr> Value; + EXPECT_FALSE(Value.has_value()); + + int A = 10; + Value = &A; + EXPECT_TRUE(Value.has_value()); + + EXPECT_EQ(*Value.value(), 10); + + Value.clear(); + EXPECT_FALSE(Value.has_value()); +} + } // end anonymous namespace >From 0b1db690d619800fc3f97516f5c2370ad4ed41e4 Mon Sep 17 00:00:00 2001 From: Benjamin Maxwell <benjamin.maxw...@arm.com> Date: Thu, 11 Sep 2025 22:01:38 +0000 Subject: [PATCH 3/5] Fixups --- ...eWithSentinel.h => OptionalWithSentinel.h} | 24 +++++++++---------- .../Target/AArch64/AArch64FrameLowering.cpp | 2 +- .../AArch64/AArch64MachineFunctionInfo.h | 10 ++++---- llvm/unittests/ADT/CMakeLists.txt | 2 +- ...lTest.cpp => OptionalWithSentinelTest.cpp} | 12 +++++----- 5 files changed, 25 insertions(+), 25 deletions(-) rename llvm/include/llvm/ADT/{ValueWithSentinel.h => OptionalWithSentinel.h} (68%) rename llvm/unittests/ADT/{ValueWithSentinelTest.cpp => OptionalWithSentinelTest.cpp} (83%) diff --git a/llvm/include/llvm/ADT/ValueWithSentinel.h b/llvm/include/llvm/ADT/OptionalWithSentinel.h similarity index 68% rename from llvm/include/llvm/ADT/ValueWithSentinel.h rename to llvm/include/llvm/ADT/OptionalWithSentinel.h index 4c45258724459..0f5126ffea12a 100644 --- a/llvm/include/llvm/ADT/ValueWithSentinel.h +++ b/llvm/include/llvm/ADT/OptionalWithSentinel.h @@ -7,13 +7,13 @@ //===----------------------------------------------------------------------===// /// /// \file -/// This file defines the ValueWithSentinel class, which is a type akin to a +/// This file defines the OptionalWithSentinel class, which is a type akin to a /// std::optional, but uses a sentinel rather than an additional "valid" flag. /// //===----------------------------------------------------------------------===// -#ifndef LLVM_ADT_VALUEWITHSENTINEL_H -#define LLVM_ADT_VALUEWITHSENTINEL_H +#ifndef LLVM_ADT_OPTIONALWITHSENTINEL_H +#define LLVM_ADT_OPTIONALWITHSENTINEL_H #include <cassert> #include <limits> @@ -21,25 +21,25 @@ namespace llvm { -template <typename T, T Sentinel> class ValueWithSentinel { +template <typename T, T Sentinel> class OptionalWithSentinel { public: - ValueWithSentinel() = default; + OptionalWithSentinel() = default; - ValueWithSentinel(T Value) : Value(std::move(Value)) { + OptionalWithSentinel(T Value) : Value(std::move(Value)) { assert(Value != Sentinel && "Value is sentinel (use default constructor)"); }; - ValueWithSentinel &operator=(const T &NewValue) { + OptionalWithSentinel &operator=(const T &NewValue) { assert(NewValue != Sentinel && "NewValue is sentinel (use .clear())"); Value = NewValue; return *this; } - bool operator==(const ValueWithSentinel &Other) const { + bool operator==(const OptionalWithSentinel &Other) const { return Value == Other.Value; } - bool operator!=(const ValueWithSentinel &Other) const { + bool operator!=(const OptionalWithSentinel &Other) const { return !(*this == Other); } @@ -48,7 +48,7 @@ template <typename T, T Sentinel> class ValueWithSentinel { return Value; } const T &value() const { - return const_cast<ValueWithSentinel &>(*this).value(); + return const_cast<OptionalWithSentinel &>(*this).value(); } T &operator*() { return value(); } @@ -66,8 +66,8 @@ template <typename T, T Sentinel> class ValueWithSentinel { }; template <typename T> -using ValueWithSentinelNumericMax = - ValueWithSentinel<T, std::numeric_limits<T>::max()>; +using OptionalWithSentinelIntMax = + OptionalWithSentinel<T, std::numeric_limits<T>::max()>; } // namespace llvm diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp index 2dcbd1b007b33..edc6fcd8aba56 100644 --- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp +++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp @@ -3479,7 +3479,7 @@ bool AArch64FrameLowering::assignCalleeSavedSpillSlots( } Register LastReg = 0; - ValueWithSentinelNumericMax<int> HazardSlotIndex; + OptionalWithSentinelIntMax<int> HazardSlotIndex; for (auto &CS : CSI) { MCRegister Reg = CS.getReg(); const TargetRegisterClass *RC = RegInfo->getMinimalPhysRegClass(Reg); diff --git a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h index def796aaaface..921918f9f05dd 100644 --- a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h +++ b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h @@ -16,9 +16,9 @@ #include "AArch64Subtarget.h" #include "Utils/AArch64SMEAttributes.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/OptionalWithSentinel.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/ValueWithSentinel.h" #include "llvm/CodeGen/CallingConvLower.h" #include "llvm/CodeGen/MIRYamlMapping.h" #include "llvm/CodeGen/MachineFrameInfo.h" @@ -39,7 +39,7 @@ class AArch64Subtarget; class MachineInstr; struct TPIDR2Object { - ValueWithSentinelNumericMax<int> FrameIndex; + OptionalWithSentinelIntMax<int> FrameIndex; unsigned Uses = 0; }; @@ -115,8 +115,8 @@ class AArch64FunctionInfo final : public MachineFunctionInfo { /// The stack slots used to add space between FPR and GPR accesses when using /// hazard padding. StackHazardCSRSlotIndex is added between GPR and FPR CSRs. /// StackHazardSlotIndex is added between (sorted) stack objects. - ValueWithSentinelNumericMax<int> StackHazardSlotIndex; - ValueWithSentinelNumericMax<int> StackHazardCSRSlotIndex; + OptionalWithSentinelIntMax<int> StackHazardSlotIndex; + OptionalWithSentinelIntMax<int> StackHazardCSRSlotIndex; /// True if this function has a subset of CSRs that is handled explicitly via /// copies. @@ -206,7 +206,7 @@ class AArch64FunctionInfo final : public MachineFunctionInfo { bool HasSwiftAsyncContext = false; /// The stack slot where the Swift asynchronous context is stored. - ValueWithSentinelNumericMax<int> SwiftAsyncContextFrameIdx; + OptionalWithSentinelIntMax<int> SwiftAsyncContextFrameIdx; bool IsMTETagged = false; diff --git a/llvm/unittests/ADT/CMakeLists.txt b/llvm/unittests/ADT/CMakeLists.txt index ef46ea2f8a083..5fec5ba16359c 100644 --- a/llvm/unittests/ADT/CMakeLists.txt +++ b/llvm/unittests/ADT/CMakeLists.txt @@ -54,6 +54,7 @@ add_llvm_unittest(ADTTests LazyAtomicPointerTest.cpp MappedIteratorTest.cpp MapVectorTest.cpp + OptionalWithSentinelTest.cpp PackedVectorTest.cpp PagedVectorTest.cpp PointerEmbeddedIntTest.cpp @@ -93,7 +94,6 @@ add_llvm_unittest(ADTTests TwineTest.cpp TypeSwitchTest.cpp TypeTraitsTest.cpp - ValueWithSentinelTest.cpp ) target_link_libraries(ADTTests PRIVATE LLVMTestingSupport) diff --git a/llvm/unittests/ADT/ValueWithSentinelTest.cpp b/llvm/unittests/ADT/OptionalWithSentinelTest.cpp similarity index 83% rename from llvm/unittests/ADT/ValueWithSentinelTest.cpp rename to llvm/unittests/ADT/OptionalWithSentinelTest.cpp index a972161450a62..88179b2527a52 100644 --- a/llvm/unittests/ADT/ValueWithSentinelTest.cpp +++ b/llvm/unittests/ADT/OptionalWithSentinelTest.cpp @@ -6,16 +6,16 @@ // //===----------------------------------------------------------------------===// -#include "llvm/ADT/ValueWithSentinel.h" +#include "llvm/ADT/OptionalWithSentinel.h" #include "gtest/gtest.h" using namespace llvm; namespace { -TEST(ValueWithSentinelTest, Basic) { +TEST(OptionalWithSentinelTest, Basic) { // Default constructor should equal sentinel. - ValueWithSentinelNumericMax<int> Value; + OptionalWithSentinelIntMax<int> Value; EXPECT_FALSE(Value.has_value()); EXPECT_FALSE(bool(Value)); @@ -35,7 +35,7 @@ TEST(ValueWithSentinelTest, Basic) { EXPECT_FALSE(bool(Value)); // construction from value, comparison operators - ValueWithSentinelNumericMax<int> OtherValue(99); + OptionalWithSentinelIntMax<int> OtherValue(99); EXPECT_TRUE(OtherValue.has_value()); EXPECT_TRUE(bool(OtherValue)); EXPECT_EQ(OtherValue, 99); @@ -45,8 +45,8 @@ TEST(ValueWithSentinelTest, Basic) { EXPECT_EQ(Value, OtherValue); } -TEST(ValueWithSentinelTest, PointerType) { - ValueWithSentinel<int *, nullptr> Value; +TEST(OptionalWithSentinelTest, PointerType) { + OptionalWithSentinel<int *, nullptr> Value; EXPECT_FALSE(Value.has_value()); int A = 10; >From 9ea7165b6be407c47dc7bd6b721e4b0fe43ddc5b Mon Sep 17 00:00:00 2001 From: Benjamin Maxwell <benjamin.maxw...@arm.com> Date: Fri, 12 Sep 2025 09:22:58 +0000 Subject: [PATCH 4/5] Rename --- ...tionalWithSentinel.h => ValueOrSentinel.h} | 23 +++++++++---------- .../Target/AArch64/AArch64FrameLowering.cpp | 2 +- .../AArch64/AArch64MachineFunctionInfo.h | 10 ++++---- llvm/unittests/ADT/CMakeLists.txt | 2 +- ...ntinelTest.cpp => ValueOrSentinelTest.cpp} | 12 +++++----- llvm/unittests/Support/CommandLineTest.cpp | 4 ++-- 6 files changed, 26 insertions(+), 27 deletions(-) rename llvm/include/llvm/ADT/{OptionalWithSentinel.h => ValueOrSentinel.h} (68%) rename llvm/unittests/ADT/{OptionalWithSentinelTest.cpp => ValueOrSentinelTest.cpp} (83%) diff --git a/llvm/include/llvm/ADT/OptionalWithSentinel.h b/llvm/include/llvm/ADT/ValueOrSentinel.h similarity index 68% rename from llvm/include/llvm/ADT/OptionalWithSentinel.h rename to llvm/include/llvm/ADT/ValueOrSentinel.h index 0f5126ffea12a..26d8fdb7b4f9d 100644 --- a/llvm/include/llvm/ADT/OptionalWithSentinel.h +++ b/llvm/include/llvm/ADT/ValueOrSentinel.h @@ -7,13 +7,13 @@ //===----------------------------------------------------------------------===// /// /// \file -/// This file defines the OptionalWithSentinel class, which is a type akin to a +/// This file defines the ValueOrSentinel class, which is a type akin to a /// std::optional, but uses a sentinel rather than an additional "valid" flag. /// //===----------------------------------------------------------------------===// -#ifndef LLVM_ADT_OPTIONALWITHSENTINEL_H -#define LLVM_ADT_OPTIONALWITHSENTINEL_H +#ifndef LLVM_ADT_VALUEORSENTINEL_H +#define LLVM_ADT_VALUEORSENTINEL_H #include <cassert> #include <limits> @@ -21,25 +21,25 @@ namespace llvm { -template <typename T, T Sentinel> class OptionalWithSentinel { +template <typename T, T Sentinel> class ValueOrSentinel { public: - OptionalWithSentinel() = default; + ValueOrSentinel() = default; - OptionalWithSentinel(T Value) : Value(std::move(Value)) { + ValueOrSentinel(T Value) : Value(std::move(Value)) { assert(Value != Sentinel && "Value is sentinel (use default constructor)"); }; - OptionalWithSentinel &operator=(const T &NewValue) { + ValueOrSentinel &operator=(const T &NewValue) { assert(NewValue != Sentinel && "NewValue is sentinel (use .clear())"); Value = NewValue; return *this; } - bool operator==(const OptionalWithSentinel &Other) const { + bool operator==(const ValueOrSentinel &Other) const { return Value == Other.Value; } - bool operator!=(const OptionalWithSentinel &Other) const { + bool operator!=(const ValueOrSentinel &Other) const { return !(*this == Other); } @@ -48,7 +48,7 @@ template <typename T, T Sentinel> class OptionalWithSentinel { return Value; } const T &value() const { - return const_cast<OptionalWithSentinel &>(*this).value(); + return const_cast<ValueOrSentinel &>(*this).value(); } T &operator*() { return value(); } @@ -66,8 +66,7 @@ template <typename T, T Sentinel> class OptionalWithSentinel { }; template <typename T> -using OptionalWithSentinelIntMax = - OptionalWithSentinel<T, std::numeric_limits<T>::max()>; +using ValueOrSentinelIntMax = ValueOrSentinel<T, std::numeric_limits<T>::max()>; } // namespace llvm diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp index edc6fcd8aba56..06e1b8788284d 100644 --- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp +++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp @@ -3479,7 +3479,7 @@ bool AArch64FrameLowering::assignCalleeSavedSpillSlots( } Register LastReg = 0; - OptionalWithSentinelIntMax<int> HazardSlotIndex; + ValueOrSentinelIntMax<int> HazardSlotIndex; for (auto &CS : CSI) { MCRegister Reg = CS.getReg(); const TargetRegisterClass *RC = RegInfo->getMinimalPhysRegClass(Reg); diff --git a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h index 921918f9f05dd..43034202acd6e 100644 --- a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h +++ b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h @@ -16,9 +16,9 @@ #include "AArch64Subtarget.h" #include "Utils/AArch64SMEAttributes.h" #include "llvm/ADT/ArrayRef.h" -#include "llvm/ADT/OptionalWithSentinel.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/ValueOrSentinel.h" #include "llvm/CodeGen/CallingConvLower.h" #include "llvm/CodeGen/MIRYamlMapping.h" #include "llvm/CodeGen/MachineFrameInfo.h" @@ -39,7 +39,7 @@ class AArch64Subtarget; class MachineInstr; struct TPIDR2Object { - OptionalWithSentinelIntMax<int> FrameIndex; + ValueOrSentinelIntMax<int> FrameIndex; unsigned Uses = 0; }; @@ -115,8 +115,8 @@ class AArch64FunctionInfo final : public MachineFunctionInfo { /// The stack slots used to add space between FPR and GPR accesses when using /// hazard padding. StackHazardCSRSlotIndex is added between GPR and FPR CSRs. /// StackHazardSlotIndex is added between (sorted) stack objects. - OptionalWithSentinelIntMax<int> StackHazardSlotIndex; - OptionalWithSentinelIntMax<int> StackHazardCSRSlotIndex; + ValueOrSentinelIntMax<int> StackHazardSlotIndex; + ValueOrSentinelIntMax<int> StackHazardCSRSlotIndex; /// True if this function has a subset of CSRs that is handled explicitly via /// copies. @@ -206,7 +206,7 @@ class AArch64FunctionInfo final : public MachineFunctionInfo { bool HasSwiftAsyncContext = false; /// The stack slot where the Swift asynchronous context is stored. - OptionalWithSentinelIntMax<int> SwiftAsyncContextFrameIdx; + ValueOrSentinelIntMax<int> SwiftAsyncContextFrameIdx; bool IsMTETagged = false; diff --git a/llvm/unittests/ADT/CMakeLists.txt b/llvm/unittests/ADT/CMakeLists.txt index 5fec5ba16359c..af79a1307f050 100644 --- a/llvm/unittests/ADT/CMakeLists.txt +++ b/llvm/unittests/ADT/CMakeLists.txt @@ -54,7 +54,7 @@ add_llvm_unittest(ADTTests LazyAtomicPointerTest.cpp MappedIteratorTest.cpp MapVectorTest.cpp - OptionalWithSentinelTest.cpp + ValueOrSentinelTest.cpp PackedVectorTest.cpp PagedVectorTest.cpp PointerEmbeddedIntTest.cpp diff --git a/llvm/unittests/ADT/OptionalWithSentinelTest.cpp b/llvm/unittests/ADT/ValueOrSentinelTest.cpp similarity index 83% rename from llvm/unittests/ADT/OptionalWithSentinelTest.cpp rename to llvm/unittests/ADT/ValueOrSentinelTest.cpp index 88179b2527a52..858fb5319e2d8 100644 --- a/llvm/unittests/ADT/OptionalWithSentinelTest.cpp +++ b/llvm/unittests/ADT/ValueOrSentinelTest.cpp @@ -6,16 +6,16 @@ // //===----------------------------------------------------------------------===// -#include "llvm/ADT/OptionalWithSentinel.h" +#include "llvm/ADT/ValueOrSentinel.h" #include "gtest/gtest.h" using namespace llvm; namespace { -TEST(OptionalWithSentinelTest, Basic) { +TEST(ValueOrSentinelTest, Basic) { // Default constructor should equal sentinel. - OptionalWithSentinelIntMax<int> Value; + ValueOrSentinelIntMax<int> Value; EXPECT_FALSE(Value.has_value()); EXPECT_FALSE(bool(Value)); @@ -35,7 +35,7 @@ TEST(OptionalWithSentinelTest, Basic) { EXPECT_FALSE(bool(Value)); // construction from value, comparison operators - OptionalWithSentinelIntMax<int> OtherValue(99); + ValueOrSentinelIntMax<int> OtherValue(99); EXPECT_TRUE(OtherValue.has_value()); EXPECT_TRUE(bool(OtherValue)); EXPECT_EQ(OtherValue, 99); @@ -45,8 +45,8 @@ TEST(OptionalWithSentinelTest, Basic) { EXPECT_EQ(Value, OtherValue); } -TEST(OptionalWithSentinelTest, PointerType) { - OptionalWithSentinel<int *, nullptr> Value; +TEST(ValueOrSentinelTest, PointerType) { + ValueOrSentinel<int *, nullptr> Value; EXPECT_FALSE(Value.has_value()); int A = 10; diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp index ad06ca91c226e..c1b4dde2bae7a 100644 --- a/llvm/unittests/Support/CommandLineTest.cpp +++ b/llvm/unittests/Support/CommandLineTest.cpp @@ -1412,7 +1412,7 @@ TEST_F(PrintOptionInfoTest, PrintOptionInfoValueOptionalWithoutSentinel) { // clang-format on } -TEST_F(PrintOptionInfoTest, PrintOptionInfoValueOptionalWithSentinel) { +TEST_F(PrintOptionInfoTest, PrintOptionInfoValueValueOrSentinel) { std::string Output = runTest( cl::ValueOptional, cl::values(clEnumValN(OptionValue::Val, "v1", "desc1"), clEnumValN(OptionValue::Val, "", ""))); @@ -1426,7 +1426,7 @@ TEST_F(PrintOptionInfoTest, PrintOptionInfoValueOptionalWithSentinel) { // clang-format on } -TEST_F(PrintOptionInfoTest, PrintOptionInfoValueOptionalWithSentinelWithHelp) { +TEST_F(PrintOptionInfoTest, PrintOptionInfoValueValueOrSentinelWithHelp) { std::string Output = runTest( cl::ValueOptional, cl::values(clEnumValN(OptionValue::Val, "v1", "desc1"), clEnumValN(OptionValue::Val, "", "desc2"))); >From b52e4c0e0e0f90464529f502348f2507d9c5502a Mon Sep 17 00:00:00 2001 From: Benjamin Maxwell <benjamin.maxw...@arm.com> Date: Fri, 12 Sep 2025 13:33:56 +0000 Subject: [PATCH 5/5] Support adjusted values --- clang/include/clang/Basic/UnsignedOrNone.h | 37 ++++---------- llvm/include/llvm/ADT/ValueOrSentinel.h | 56 ++++++++++++++-------- 2 files changed, 46 insertions(+), 47 deletions(-) diff --git a/clang/include/clang/Basic/UnsignedOrNone.h b/clang/include/clang/Basic/UnsignedOrNone.h index 659fd8c6487d2..e0104c8b34474 100644 --- a/clang/include/clang/Basic/UnsignedOrNone.h +++ b/clang/include/clang/Basic/UnsignedOrNone.h @@ -14,39 +14,22 @@ #ifndef LLVM_CLANG_BASIC_UNSIGNED_OR_NONE_H #define LLVM_CLANG_BASIC_UNSIGNED_OR_NONE_H -#include <cassert> -#include <optional> +#include "llvm/ADT/ValueOrSentinel.h" namespace clang { -struct UnsignedOrNone { - constexpr UnsignedOrNone(std::nullopt_t) : Rep(0) {} - UnsignedOrNone(unsigned Val) : Rep(Val + 1) { assert(operator bool()); } - UnsignedOrNone(int) = delete; - - constexpr static UnsignedOrNone fromInternalRepresentation(unsigned Rep) { - return {std::nullopt, Rep}; - } - constexpr unsigned toInternalRepresentation() const { return Rep; } - - explicit constexpr operator bool() const { return Rep != 0; } - unsigned operator*() const { - assert(operator bool()); - return Rep - 1; - } - - friend constexpr bool operator==(UnsignedOrNone LHS, UnsignedOrNone RHS) { - return LHS.Rep == RHS.Rep; +namespace detail { +struct AdjustAddOne { + constexpr static unsigned toRepresentation(unsigned Value) { + return Value + 1; } - friend constexpr bool operator!=(UnsignedOrNone LHS, UnsignedOrNone RHS) { - return LHS.Rep != RHS.Rep; + constexpr static unsigned fromRepresentation(unsigned Value) { + return Value - 1; } - -private: - constexpr UnsignedOrNone(std::nullopt_t, unsigned Rep) : Rep(Rep) {}; - - unsigned Rep; }; +} // namespace detail + +using UnsignedOrNone = llvm::ValueOrSentinel<unsigned, 0, detail::AdjustAddOne>; } // namespace clang diff --git a/llvm/include/llvm/ADT/ValueOrSentinel.h b/llvm/include/llvm/ADT/ValueOrSentinel.h index 26d8fdb7b4f9d..5f4a80f2bc1a4 100644 --- a/llvm/include/llvm/ADT/ValueOrSentinel.h +++ b/llvm/include/llvm/ADT/ValueOrSentinel.h @@ -17,51 +17,67 @@ #include <cassert> #include <limits> +#include <optional> #include <utility> namespace llvm { -template <typename T, T Sentinel> class ValueOrSentinel { +namespace detail { +/// An adjustment allows changing how the value is stored. An example use case +/// is to use zero as a sentinel value. +template <typename T> struct DefaultValueAdjustment { + constexpr static T toRepresentation(T Value) { return Value; } + constexpr static T fromRepresentation(T Value) { return Value; } +}; +} // namespace detail + +template <typename T, T Sentinel, + typename Adjust = detail::DefaultValueAdjustment<T>> +class ValueOrSentinel { public: - ValueOrSentinel() = default; + constexpr ValueOrSentinel() = default; + constexpr ValueOrSentinel(std::nullopt_t){}; - ValueOrSentinel(T Value) : Value(std::move(Value)) { - assert(Value != Sentinel && "Value is sentinel (use default constructor)"); + constexpr ValueOrSentinel(T Value) : Value(Adjust::toRepresentation(Value)) { + assert(this->Value != Sentinel && + "Value is sentinel (use default constructor)"); }; - ValueOrSentinel &operator=(const T &NewValue) { - assert(NewValue != Sentinel && "NewValue is sentinel (use .clear())"); - Value = NewValue; + constexpr ValueOrSentinel &operator=(T NewValue) { + Value = Adjust::toRepresentation(NewValue); + assert(Value != Sentinel && "NewValue is sentinel (use .clear())"); return *this; } - bool operator==(const ValueOrSentinel &Other) const { + constexpr bool operator==(ValueOrSentinel Other) const { return Value == Other.Value; } - bool operator!=(const ValueOrSentinel &Other) const { + constexpr bool operator!=(ValueOrSentinel Other) const { return !(*this == Other); } - T &value() { + T value() const { assert(has_value() && ".value() called on sentinel"); - return Value; + return Adjust::fromRepresentation(Value); } - const T &value() const { - return const_cast<ValueOrSentinel &>(*this).value(); - } - - T &operator*() { return value(); } - const T &operator*() const { return value(); } + T operator*() const { return value(); } bool has_value() const { return Value != Sentinel; } - explicit operator bool() const { return has_value(); } explicit operator T() const { return value(); } + explicit constexpr operator bool() const { return has_value(); } + + constexpr void clear() { Value = Sentinel; } + + constexpr static ValueOrSentinel fromInternalRepresentation(unsigned Value) { + return {std::nullopt, Value}; + } + constexpr T toInternalRepresentation() const { return Value; } - void clear() { Value = Sentinel; } +protected: + ValueOrSentinel(std::nullopt_t, T Value) : Value(Value){}; -private: T Value{Sentinel}; }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
