[PATCH] D135488: [codegen] Display stack layouts in console

Tue Nov 15 14:22:01 PST 2022

paulkirth added a comment.

Thanks for the feedback! I have a few questions I'm hoping you can answer.

In D135488#3928559 <https://reviews.llvm.org/D135488#3928559>, @arsenm wrote:

> I don't think we should be pointing users to -mllvm flags. Plus, I don't really think random dbgs() printing is going to interact correctly with other diagnostics

I modeled this pass off of the MachineFunctionPrinterPass, does that mean it it should also use a different stream? This seems to be a fairly common pattern, so should we be filing bugs and tracking work in this area?

In D135488#3928580 <https://reviews.llvm.org/D135488#3928580>, @arsenm wrote:

> I'd be a bit more comfortable routing this through the backend remarks infrastructure, although it's a lot bigger than everything else currently reported there

I'm not sure that this diagnostic belongs in optimization remarks, though. It isn't describing any of the decision making that went into the stack layout, which is what I think most remarks typically describe. I'm basing that on https://clang.llvm.org/docs/UsersManual.html#options-to-emit-optimization-reports. Is my interpretation of that too narrow?

Also, is there something special about the remarks output that makes it better? Is the setup/initialization more careful than for the other streams? I'd like to understand the trade-off a bit better. Our documentation makes it seem as though its geared towards compiler engineers, where I view this as a more general diagnostic output, like the other printing passes.

================
Comment at: llvm/lib/CodeGen/StackFramePrinterPass.cpp:120-122
+      // This is a dead slot, so skip it
+      if (Size == ~0ULL)
+        continue;
----------------
arsenm wrote:
> Check MFI.isDead instead?
Oh, good suggestion. That check bothered me, but I missed that API. I'll update this patch to reflect your suggestion.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135488/new/

https://reviews.llvm.org/D135488