[llvm] dca40e3 - [CodeGen] Replace CCValAssign::Loc with std::variant (NFCI)
Sergei Barannikov via llvm-commits
llvm-commits at lists.llvm.org
Sun Jan 15 00:07:35 PST 2023
Author: Sergei Barannikov
Date: 2023-01-15T10:31:42+03:00
New Revision: dca40e3288f481cda1ce37c647b782a3b468b7d1
URL: https://github.com/llvm/llvm-project/commit/dca40e3288f481cda1ce37c647b782a3b468b7d1
DIFF: https://github.com/llvm/llvm-project/commit/dca40e3288f481cda1ce37c647b782a3b468b7d1.diff
LOG: [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
Added:
Modified:
llvm/include/llvm/CodeGen/CallingConvLower.h
llvm/lib/CodeGen/CallingConvLower.cpp
llvm/lib/Target/Sparc/SparcISelLowering.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/CallingConvLower.h b/llvm/include/llvm/CodeGen/CallingConvLower.h
index 5c3776e972c04..005cfd269e3cf 100644
--- a/llvm/include/llvm/CodeGen/CallingConvLower.h
+++ b/llvm/include/llvm/CodeGen/CallingConvLower.h
@@ -52,15 +52,16 @@ class CCValAssign {
};
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<Register, int64_t, unsigned> 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 @@ class CCValAssign {
/// 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
diff erentiate between a pending CCValAssign and other
- // kinds, as they are stored in a
diff erent 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<Register>(Data); }
+ bool isMemLoc() const { return std::holds_alternative<int64_t>(Data); }
+ bool isPendingLoc() const { return std::holds_alternative<unsigned>(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<Register>(Data); }
+ unsigned getLocMemOffset() const { return std::get<int64_t>(Data); }
+ unsigned getExtraInfo() const { return std::get<unsigned>(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 f49cc6427f067..ce1ef571c9df1 100644
--- a/llvm/lib/CodeGen/CallingConvLower.cpp
+++ b/llvm/lib/CodeGen/CallingConvLower.cpp
@@ -231,7 +231,7 @@ void CCState::getRemainingRegParmsForType(SmallVectorImpl<MCPhysReg> &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 af678fd51a90c..0672b294d0a58 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<CCValAssign> &ArgLocs,
ArrayRef<ISD::OutputArg> 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<CCValAssign> &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<CCValAssign> &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;
}
}
More information about the llvm-commits
mailing list