[llvm] [NFC] [DSE] Refactor DSE (PR #100956)
Haopeng Liu via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 19 14:10:13 PDT 2024
================
@@ -806,6 +806,38 @@ bool canSkipDef(MemoryDef *D, bool DefVisibleToCaller) {
return false;
}
+// A memory location wrapper that represents a MemoryLocation, `MemLoc`,
+// defined by `MemDef`.
+class MemoryLocationWrapper {
+public:
+ MemoryLocationWrapper(MemoryLocation MemLoc, MemoryDef *MemDef)
+ : MemLoc(MemLoc), MemDef(MemDef) {
+ assert(MemLoc.Ptr && "MemLoc should be not null");
+ UnderlyingObject = getUnderlyingObject(MemLoc.Ptr);
+ DefInst = MemDef->getMemoryInst();
+ }
+
+ MemoryAccess *GetDefiningAccess() const {
+ return MemDef->getDefiningAccess();
+ }
+
+ MemoryLocation MemLoc;
+ const Value *UnderlyingObject;
+ MemoryDef *MemDef;
+ Instruction *DefInst;
+};
+
+// A memory def wrapper that represents a MemoryDef and the MemoryLocation(s)
+// defined by this MemoryDef.
+class MemoryDefWrapper {
+public:
+ MemoryDefWrapper(MemoryDef *MemDef, std::optional<MemoryLocation> MemLoc) {
+ if (MemLoc.has_value())
+ DefinedLocation = MemoryLocationWrapper(*MemLoc, MemDef);
+ }
+ std::optional<MemoryLocationWrapper> DefinedLocation = std::nullopt;
----------------
haopliu wrote:
> I can see that there's a lot of parameters for getDomMemoryDef and the relationships are loose and only tied together by the same name prefix "Killing"
This is the main reason. I tried to make `getDomMemoryDef` with a bit clearer interface: given a (killing) MemoryLocWrapper, return a MemoryDef that dominates the MemoryLoc or underlying object. This process needs DefInst but mostly for additional checks, I think, so I choose `MemoryLocWrapper` as a parameter and try to bundle all killing-related stuff in this parameter.
Thanks for raising this concern! I agree this bundling is too surface. May be an ideal way is to refactor/divide the `getDomMemoryDef` logic then bundle the members and the corresponding logic together.
How about we step back and keep `getDomMemoryDef` as it was?
https://github.com/llvm/llvm-project/pull/100956
More information about the llvm-commits
mailing list