From dca40e3288f481cda1ce37c647b782a3b468b7d1 Mon Sep 17 00:00:00 2001 From: Sergei Barannikov Date: Sun, 15 Jan 2023 10:31:07 +0300 Subject: [PATCH] [CodeGen] Replace CCValAssign::Loc with std::variant (NFCI) The motivation behind this change is as follows. Targets with stack growing up (there are no such in-tree targets) pass arguments at negative offsets relative to the stack pointer. This makes it hard to use the generic value assigner because CCValAssign stores the offset as an unsigned integer, which is then zero-extended when converted to int64_t, e.g. when passing to `CreateFixedObject`. This results in conversion of, for example, -4 into 4294967292, which is not desired. While it is possible to insert a cast to `int` before passing the result of `getLocMemOffset` into `CreateFixedObject` in backend code, this is error-prone, and some uses of `getLocMemOffset` are located in places common to all backends (e.g. `CallLowering::handleAssignments`). That said, I wanted to change the type of the memory offset from `unsigned` to `int64_t` (this would be consistent with other places where stack offsets are used). However, the `Loc` field which stores the offset is shared between three different kinds of the location: register, memory, and "pending". Storing a register number as `int64_t` does not seem right (there are `Register` and `MCRegister` for this), so I did the most straightforward change - replaced the `Loc` field with std::variant. The main change that changes the type of the memory offset from `unsigned` to `int64_t` will be in a follow-up patch to simplify the review. Reviewed By: MaskRay, nikic Differential Revision: https://reviews.llvm.org/D136043 --- llvm/include/llvm/CodeGen/CallingConvLower.h | 97 +++++++++++----------------- llvm/lib/CodeGen/CallingConvLower.cpp | 31 ++++----- llvm/lib/Target/Sparc/SparcISelLowering.cpp | 17 ++--- 3 files changed, 61 insertions(+), 84 deletions(-) diff --git a/llvm/include/llvm/CodeGen/CallingConvLower.h b/llvm/include/llvm/CodeGen/CallingConvLower.h index 5c3776e..005cfd2 100644 --- a/llvm/include/llvm/CodeGen/CallingConvLower.h +++ b/llvm/include/llvm/CodeGen/CallingConvLower.h @@ -52,15 +52,16 @@ public: }; private: + // Holds one of: + // - the register that the value is assigned to; + // - the memory offset at which the value resides; + // - additional information about pending location; the exact interpretation + // of the data is target-dependent. + std::variant Data; + /// ValNo - This is the value number being assigned (e.g. an argument number). unsigned ValNo; - /// Loc is either a stack offset or a register number. - unsigned Loc; - - /// isMem - True if this is a memory loc, false if it is a register loc. - unsigned isMem : 1; - /// isCustom - True if this arg/retval requires special handling. unsigned isCustom : 1; @@ -72,82 +73,60 @@ private: /// LocVT - The type of the location being assigned to. MVT LocVT; -public: - static CCValAssign getReg(unsigned ValNo, MVT ValVT, - unsigned RegNo, MVT LocVT, - LocInfo HTP) { - CCValAssign Ret; - Ret.ValNo = ValNo; - Ret.Loc = RegNo; - Ret.isMem = false; - Ret.isCustom = false; - Ret.HTP = HTP; - Ret.ValVT = ValVT; - Ret.LocVT = LocVT; - return Ret; + CCValAssign(LocInfo HTP, unsigned ValNo, MVT ValVT, MVT LocVT, bool IsCustom) + : ValNo(ValNo), isCustom(IsCustom), HTP(HTP), ValVT(ValVT), LocVT(LocVT) { } - static CCValAssign getCustomReg(unsigned ValNo, MVT ValVT, - unsigned RegNo, MVT LocVT, - LocInfo HTP) { - CCValAssign Ret; - Ret = getReg(ValNo, ValVT, RegNo, LocVT, HTP); - Ret.isCustom = true; +public: + static CCValAssign getReg(unsigned ValNo, MVT ValVT, unsigned RegNo, + MVT LocVT, LocInfo HTP, bool IsCustom = false) { + CCValAssign Ret(HTP, ValNo, ValVT, LocVT, IsCustom); + Ret.Data = Register(RegNo); return Ret; } - static CCValAssign getMem(unsigned ValNo, MVT ValVT, - unsigned Offset, MVT LocVT, - LocInfo HTP) { - CCValAssign Ret; - Ret.ValNo = ValNo; - Ret.Loc = Offset; - Ret.isMem = true; - Ret.isCustom = false; - Ret.HTP = HTP; - Ret.ValVT = ValVT; - Ret.LocVT = LocVT; - return Ret; + static CCValAssign getCustomReg(unsigned ValNo, MVT ValVT, unsigned RegNo, + MVT LocVT, LocInfo HTP) { + return getReg(ValNo, ValVT, RegNo, LocVT, HTP, /*IsCustom=*/true); } - static CCValAssign getCustomMem(unsigned ValNo, MVT ValVT, - unsigned Offset, MVT LocVT, - LocInfo HTP) { - CCValAssign Ret; - Ret = getMem(ValNo, ValVT, Offset, LocVT, HTP); - Ret.isCustom = true; + static CCValAssign getMem(unsigned ValNo, MVT ValVT, unsigned Offset, + MVT LocVT, LocInfo HTP, bool IsCustom = false) { + CCValAssign Ret(HTP, ValNo, ValVT, LocVT, IsCustom); + Ret.Data = int64_t(Offset); return Ret; } - // There is no need to differentiate between a pending CCValAssign and other - // kinds, as they are stored in a different list. + static CCValAssign getCustomMem(unsigned ValNo, MVT ValVT, unsigned Offset, + MVT LocVT, LocInfo HTP) { + return getMem(ValNo, ValVT, Offset, LocVT, HTP, /*IsCustom=*/true); + } + static CCValAssign getPending(unsigned ValNo, MVT ValVT, MVT LocVT, LocInfo HTP, unsigned ExtraInfo = 0) { - return getReg(ValNo, ValVT, ExtraInfo, LocVT, HTP); + CCValAssign Ret(HTP, ValNo, ValVT, LocVT, false); + Ret.Data = ExtraInfo; + return Ret; } - void convertToReg(unsigned RegNo) { - Loc = RegNo; - isMem = false; - } + void convertToReg(unsigned RegNo) { Data = Register(RegNo); } - void convertToMem(unsigned Offset) { - Loc = Offset; - isMem = true; - } + void convertToMem(unsigned Offset) { Data = int64_t(Offset); } unsigned getValNo() const { return ValNo; } MVT getValVT() const { return ValVT; } - bool isRegLoc() const { return !isMem; } - bool isMemLoc() const { return isMem; } + bool isRegLoc() const { return std::holds_alternative(Data); } + bool isMemLoc() const { return std::holds_alternative(Data); } + bool isPendingLoc() const { return std::holds_alternative(Data); } bool needsCustom() const { return isCustom; } - Register getLocReg() const { assert(isRegLoc()); return Loc; } - unsigned getLocMemOffset() const { assert(isMemLoc()); return Loc; } - unsigned getExtraInfo() const { return Loc; } + Register getLocReg() const { return std::get(Data); } + unsigned getLocMemOffset() const { return std::get(Data); } + unsigned getExtraInfo() const { return std::get(Data); } + MVT getLocVT() const { return LocVT; } LocInfo getLocInfo() const { return HTP; } diff --git a/llvm/lib/CodeGen/CallingConvLower.cpp b/llvm/lib/CodeGen/CallingConvLower.cpp index f49cc64..ce1ef57 100644 --- a/llvm/lib/CodeGen/CallingConvLower.cpp +++ b/llvm/lib/CodeGen/CallingConvLower.cpp @@ -231,7 +231,7 @@ void CCState::getRemainingRegParmsForType(SmallVectorImpl &Regs, // when i64 and f64 are both passed in GPRs. StackOffset = SavedStackOffset; MaxStackArgAlign = SavedMaxStackArgAlign; - Locs.resize(NumLocs); + Locs.truncate(NumLocs); } void CCState::analyzeMustTailForwardedRegisters( @@ -270,19 +270,20 @@ bool CCState::resultsCompatible(CallingConv::ID CalleeCC, CCState CCInfo2(CallerCC, false, MF, RVLocs2, C); CCInfo2.AnalyzeCallResult(Ins, CallerFn); - if (RVLocs1.size() != RVLocs2.size()) - return false; - for (unsigned I = 0, E = RVLocs1.size(); I != E; ++I) { - const CCValAssign &Loc1 = RVLocs1[I]; - const CCValAssign &Loc2 = RVLocs2[I]; - - if ( // Must both be in registers, or both in memory - Loc1.isRegLoc() != Loc2.isRegLoc() || - // Must fill the same part of their locations - Loc1.getLocInfo() != Loc2.getLocInfo() || - // Memory offset/register number must be the same - Loc1.getExtraInfo() != Loc2.getExtraInfo()) + auto AreCompatible = [](const CCValAssign &Loc1, const CCValAssign &Loc2) { + assert(!Loc1.isPendingLoc() && !Loc2.isPendingLoc() && + "The location must have been decided by now"); + // Must fill the same part of their locations. + if (Loc1.getLocInfo() != Loc2.getLocInfo()) return false; - } - return true; + // Must both be in the same registers, or both in memory at the same offset. + if (Loc1.isRegLoc() && Loc2.isRegLoc()) + return Loc1.getLocReg() == Loc2.getLocReg(); + if (Loc1.isMemLoc() && Loc2.isMemLoc()) + return Loc1.getLocMemOffset() == Loc2.getLocMemOffset(); + llvm_unreachable("Unknown location kind"); + }; + + return std::equal(RVLocs1.begin(), RVLocs1.end(), RVLocs2.begin(), + RVLocs2.end(), AreCompatible); } diff --git a/llvm/lib/Target/Sparc/SparcISelLowering.cpp b/llvm/lib/Target/Sparc/SparcISelLowering.cpp index af678fd..0672b29 100644 --- a/llvm/lib/Target/Sparc/SparcISelLowering.cpp +++ b/llvm/lib/Target/Sparc/SparcISelLowering.cpp @@ -1142,7 +1142,7 @@ Register SparcTargetLowering::getRegisterByName(const char* RegName, LLT VT, static void fixupVariableFloatArgs(SmallVectorImpl &ArgLocs, ArrayRef Outs) { for (unsigned i = 0, e = ArgLocs.size(); i != e; ++i) { - const CCValAssign &VA = ArgLocs[i]; + CCValAssign &VA = ArgLocs[i]; MVT ValTy = VA.getLocVT(); // FIXME: What about f32 arguments? C promotes them to f64 when calling // varargs functions. @@ -1153,8 +1153,6 @@ static void fixupVariableFloatArgs(SmallVectorImpl &ArgLocs, continue; // This floating point argument should be reassigned. - CCValAssign NewVA; - // Determine the offset into the argument array. Register firstReg = (ValTy == MVT::f64) ? SP::D0 : SP::Q0; unsigned argSize = (ValTy == MVT::f64) ? 8 : 16; @@ -1166,21 +1164,20 @@ static void fixupVariableFloatArgs(SmallVectorImpl &ArgLocs, unsigned IReg = SP::I0 + Offset/8; if (ValTy == MVT::f64) // Full register, just bitconvert into i64. - NewVA = CCValAssign::getReg(VA.getValNo(), VA.getValVT(), - IReg, MVT::i64, CCValAssign::BCvt); + VA = CCValAssign::getReg(VA.getValNo(), VA.getValVT(), IReg, MVT::i64, + CCValAssign::BCvt); else { assert(ValTy == MVT::f128 && "Unexpected type!"); // Full register, just bitconvert into i128 -- We will lower this into // two i64s in LowerCall_64. - NewVA = CCValAssign::getCustomReg(VA.getValNo(), VA.getValVT(), - IReg, MVT::i128, CCValAssign::BCvt); + VA = CCValAssign::getCustomReg(VA.getValNo(), VA.getValVT(), IReg, + MVT::i128, CCValAssign::BCvt); } } else { // This needs to go to memory, we're out of integer registers. - NewVA = CCValAssign::getMem(VA.getValNo(), VA.getValVT(), - Offset, VA.getLocVT(), VA.getLocInfo()); + VA = CCValAssign::getMem(VA.getValNo(), VA.getValVT(), Offset, + VA.getLocVT(), VA.getLocInfo()); } - ArgLocs[i] = NewVA; } } -- 2.7.4