paulkirth added inline comments.
================
Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:44
+
+const char *PassName = "stack-frame-layout";
+
----------------
nickdesaulniers wrote:
> Consider replacing uses of `PassName` with `DEBUG_TYPE` since they have the
> same value.
ugh, I forgot to update that after adding DEBUG_TYPE. Thanks for pointing that
out.
================
Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:106
+ SlotDbgMap SlotMap = genSlotDbgMapping(MF);
+ emitStackFrameLayoutRemarks(MF, SlotMap, Rem);
+
----------------
nickdesaulniers wrote:
> can you sink the call to `genSlotDbgMapping()` into this arg list? `SlotMap`
> seems unreferenced otherwise.
surprisingly this resulted in a compiler error:
```
StackFrameLayoutAnalysisPass.cpp:104:37: error: non-const lvalue reference to
type 'SmallDenseMap<...>' cannot bind to a temporary of type
'SmallDenseMap<...>'
emitStackFrameLayoutRemarks(MF, genSlotDbgMapping(MF), Rem);
```
So I've just moved it into `emitStackFrameLayoutRemarks()` and dropped the
parameter, which avoids the issue entirely. Since SlotMap is only used there
now, it makes more sense to structure the code like this anyway.
================
Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:216
+ SlotDbgMap genSlotDbgMapping(MachineFunction &MF) {
+ SlotDbgMap SlotDebugMap;
+ SmallVector<MachineInstr *> Dbg;
----------------
nickdesaulniers wrote:
> If you sink this def into the for loop, then you don't need to clear it.
for some reason I was under the impression that our style guidelines prefered
using clear in situations like this, but I can't find it so I think my brain
tricked me. Thanks for pointing that out.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135488/new/
https://reviews.llvm.org/D135488
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
