From 800245013f60947c5983c39f79e1d32756fc117c Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Sun, 28 Sep 2025 21:36:03 +0800 Subject: [PATCH] [CodeGen] Get rid of incorrect `std` template specializations (#160804) This is a backport of llvm a558d6560437 to release/18.x, with slight context adjustment due to llvm 356be1a36ecc not being present. > This patch renames comparators > - from `std::equal_to` to > `llvm::rdf::RegisterRefEqualTo`, and > - from `std::less` to > `llvm::rdf::RegisterRefLess`. > > The original specializations don't satisfy the requirements for the > original `std` templates by being stateful and > non-default-constructible, so they make the program have UB due to C++17 > [namespace.std]/2, C++20/23 [namespace.std]/5. > > > A program may explicitly instantiate a class template defined in the > standard library only if the declaration > > - depends on the name of at least one program-defined type, and > > - the instantiation meets the standard library requirements for the > original template. This also contains a follow-up fix from llvm 99f296d2a811: > [CodeGen][test] Fix Buildbot failure due to uninitialized variables (#161085) > > Under some options used by LLVM Buildbot, an uninitialized variable > (recently added to the test suite) caused constant evaluation failure, > despite the type of that variable is an empty class. > > This PR explicitly initializes the variables with `{}` to fix the error. > Follows-up a558d656043734cc4d02e0a0a12e4c308c28f8c7. --- llvm/include/llvm/CodeGen/RDFGraph.h | 2 +- llvm/include/llvm/CodeGen/RDFRegisters.h | 54 ++++++++++++----------- llvm/lib/CodeGen/RDFLiveness.cpp | 9 ++-- llvm/lib/Target/Hexagon/RDFCopy.cpp | 2 +- llvm/lib/Target/Hexagon/RDFCopy.h | 8 ++-- llvm/unittests/CodeGen/TypeTraitsTest.cpp | 36 +++++++++++++++ 6 files changed, 75 insertions(+), 36 deletions(-) diff --git a/llvm/include/llvm/CodeGen/RDFGraph.h b/llvm/include/llvm/CodeGen/RDFGraph.h index cf7344e8c3e7..30c38da96c11 100644 --- a/llvm/include/llvm/CodeGen/RDFGraph.h +++ b/llvm/include/llvm/CodeGen/RDFGraph.h @@ -447,7 +447,7 @@ private: AllocatorTy MemPool; }; -using RegisterSet = std::set; +using RegisterSet = std::set; struct TargetOperandInfo { TargetOperandInfo(const TargetInstrInfo &tii) : TII(tii) {} diff --git a/llvm/include/llvm/CodeGen/RDFRegisters.h b/llvm/include/llvm/CodeGen/RDFRegisters.h index 7eed0b4e1e7b..c5060f00a088 100644 --- a/llvm/include/llvm/CodeGen/RDFRegisters.h +++ b/llvm/include/llvm/CodeGen/RDFRegisters.h @@ -201,6 +201,33 @@ private: std::vector AliasInfos; }; +struct RegisterRefEqualTo { + constexpr RegisterRefEqualTo(const llvm::rdf::PhysicalRegisterInfo &pri) + : PRI(&pri) {} + + bool operator()(llvm::rdf::RegisterRef A, llvm::rdf::RegisterRef B) const { + return PRI->equal_to(A, B); + } + +private: + // Make it a pointer just in case. See comment in `RegisterRefLess` below. + const llvm::rdf::PhysicalRegisterInfo *PRI; +}; + +struct RegisterRefLess { + constexpr RegisterRefLess(const llvm::rdf::PhysicalRegisterInfo &pri) + : PRI(&pri) {} + + bool operator()(llvm::rdf::RegisterRef A, llvm::rdf::RegisterRef B) const { + return PRI->less(A, B); + } + +private: + // Make it a pointer because apparently some versions of MSVC use std::swap + // on the comparator object. + const llvm::rdf::PhysicalRegisterInfo *PRI; +}; + struct RegisterAggr { RegisterAggr(const PhysicalRegisterInfo &pri) : Units(pri.getTRI().getNumRegUnits()), PRI(pri) {} @@ -336,18 +363,6 @@ template <> struct hash { } }; -template <> struct equal_to { - constexpr equal_to(const llvm::rdf::PhysicalRegisterInfo &pri) : PRI(&pri) {} - - bool operator()(llvm::rdf::RegisterRef A, llvm::rdf::RegisterRef B) const { - return PRI->equal_to(A, B); - } - -private: - // Make it a pointer just in case. See comment in `less` below. - const llvm::rdf::PhysicalRegisterInfo *PRI; -}; - template <> struct equal_to { bool operator()(const llvm::rdf::RegisterAggr &A, const llvm::rdf::RegisterAggr &B) const { @@ -355,23 +370,10 @@ template <> struct equal_to { } }; -template <> struct less { - constexpr less(const llvm::rdf::PhysicalRegisterInfo &pri) : PRI(&pri) {} - - bool operator()(llvm::rdf::RegisterRef A, llvm::rdf::RegisterRef B) const { - return PRI->less(A, B); - } - -private: - // Make it a pointer because apparently some versions of MSVC use std::swap - // on the std::less specialization. - const llvm::rdf::PhysicalRegisterInfo *PRI; -}; - } // namespace std namespace llvm::rdf { -using RegisterSet = std::set>; +using RegisterSet = std::set; } // namespace llvm::rdf #endif // LLVM_CODEGEN_RDFREGISTERS_H diff --git a/llvm/lib/CodeGen/RDFLiveness.cpp b/llvm/lib/CodeGen/RDFLiveness.cpp index 11f3fedaa5f9..43bfbad39775 100644 --- a/llvm/lib/CodeGen/RDFLiveness.cpp +++ b/llvm/lib/CodeGen/RDFLiveness.cpp @@ -655,8 +655,9 @@ void Liveness::computePhiInfo() { // defs, cache the result of subtracting these defs from a given register // ref. using RefHash = std::hash; - using RefEqual = std::equal_to; - using SubMap = std::unordered_map; + using RefEqual = RegisterRefEqualTo; + using SubMap = + std::unordered_map; std::unordered_map Subs; auto ClearIn = [](RegisterRef RR, const RegisterAggr &Mid, SubMap &SM) { if (Mid.empty()) @@ -873,7 +874,7 @@ void Liveness::computeLiveIns() { std::vector LV; for (const MachineBasicBlock::RegisterMaskPair &LI : B.liveins()) LV.push_back(RegisterRef(LI.PhysReg, LI.LaneMask)); - llvm::sort(LV, std::less(PRI)); + llvm::sort(LV, RegisterRefLess(PRI)); dbgs() << printMBBReference(B) << "\t rec = {"; for (auto I : LV) dbgs() << ' ' << Print(I, DFG); @@ -883,7 +884,7 @@ void Liveness::computeLiveIns() { LV.clear(); for (RegisterRef RR : LiveMap[&B].refs()) LV.push_back(RR); - llvm::sort(LV, std::less(PRI)); + llvm::sort(LV, RegisterRefLess(PRI)); dbgs() << "\tcomp = {"; for (auto I : LV) dbgs() << ' ' << Print(I, DFG); diff --git a/llvm/lib/Target/Hexagon/RDFCopy.cpp b/llvm/lib/Target/Hexagon/RDFCopy.cpp index b26821cd0171..b2c1075244ae 100644 --- a/llvm/lib/Target/Hexagon/RDFCopy.cpp +++ b/llvm/lib/Target/Hexagon/RDFCopy.cpp @@ -109,7 +109,7 @@ bool CopyPropagation::scanBlock(MachineBasicBlock *B) { for (NodeAddr IA : BA.Addr->members(DFG)) { if (DFG.IsCode(IA)) { NodeAddr SA = IA; - EqualityMap EM(std::less(DFG.getPRI())); + EqualityMap EM(RegisterRefLess(DFG.getPRI())); if (interpretAsCopy(SA.Addr->getCode(), EM)) recordCopy(SA, EM); } diff --git a/llvm/lib/Target/Hexagon/RDFCopy.h b/llvm/lib/Target/Hexagon/RDFCopy.h index e4fb89892831..92b2c6598265 100644 --- a/llvm/lib/Target/Hexagon/RDFCopy.h +++ b/llvm/lib/Target/Hexagon/RDFCopy.h @@ -25,8 +25,8 @@ class MachineInstr; namespace rdf { struct CopyPropagation { - CopyPropagation(DataFlowGraph &dfg) : MDT(dfg.getDT()), DFG(dfg), - RDefMap(std::less(DFG.getPRI())) {} + CopyPropagation(DataFlowGraph &dfg) + : MDT(dfg.getDT()), DFG(dfg), RDefMap(RegisterRefLess(DFG.getPRI())) {} virtual ~CopyPropagation() = default; @@ -35,7 +35,7 @@ namespace rdf { bool trace() const { return Trace; } DataFlowGraph &getDFG() { return DFG; } - using EqualityMap = std::map; + using EqualityMap = std::map; virtual bool interpretAsCopy(const MachineInstr *MI, EqualityMap &EM); private: @@ -45,7 +45,7 @@ namespace rdf { bool Trace = false; // map: register -> (map: stmt -> reaching def) - std::map> RDefMap; + std::map, RegisterRefLess> RDefMap; // map: statement -> (map: dst reg -> src reg) std::map CopyMap; std::vector Copies; diff --git a/llvm/unittests/CodeGen/TypeTraitsTest.cpp b/llvm/unittests/CodeGen/TypeTraitsTest.cpp index 0ca4fa4e82c7..c32ae155f7a1 100644 --- a/llvm/unittests/CodeGen/TypeTraitsTest.cpp +++ b/llvm/unittests/CodeGen/TypeTraitsTest.cpp @@ -6,13 +6,16 @@ // //===----------------------------------------------------------------------===// +#include "llvm/CodeGen/RDFRegisters.h" #include "llvm/CodeGen/RegisterPressure.h" #include "llvm/CodeGen/ScheduleDAG.h" #include "llvm/CodeGen/SelectionDAGNodes.h" #include "llvm/CodeGen/SlotIndexes.h" #include "llvm/CodeGen/TargetPassConfig.h" #include "gtest/gtest.h" +#include #include +#include using namespace llvm; @@ -24,5 +27,38 @@ static_assert(std::is_trivially_copyable_v, "trivially copyable"); static_assert(std::is_trivially_copyable_v, "trivially copyable"); static_assert(std::is_trivially_copyable_v, "trivially copyable"); + +// https://llvm.org/PR105169 +// Verify that we won't accidently specialize std::less and std::equal_to in a +// wrong way. +// C++17 [namespace.std]/2, C++20/23 [namespace.std]/5: +// A program may explicitly instantiate a template defined in the standard +// library only if the declaration +// - depends on the name of a user-defined type and +// - the instantiation meets the standard library requirements for the +// original template. +template constexpr bool CheckStdCmpRequirements() { + // std::less and std::equal_to are literal, default constructible, and + // copyable classes. + Fn f1{}; + auto f2 = f1; + auto f3 = std::move(f2); + f2 = f3; + f2 = std::move(f3); + + // Properties held on all known implementations, although not guaranteed by + // the standard. + static_assert(std::is_empty_v); + static_assert(std::is_trivially_default_constructible_v); + static_assert(std::is_trivially_copyable_v); + + return true; +} + +static_assert(CheckStdCmpRequirements>(), + "same as the original template"); +static_assert(CheckStdCmpRequirements>(), + "same as the original template"); + #endif -- 2.54.0