[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added inline comments.


================
Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:92
+                                               &MF.front())
+             << "FunctionName: " << ore::NV("FunctionName", MF.getName());
+    });
----------------
Why are we emitting the function name? In the serialized remarks (`-fsave-optimization-record`) it comes in the `Function` field, and in the diagnostics (`-Rpass*`) it uses debug info to show the source around it.

if it's for testing only, you can test using the serialized remarks with YAML.


================
Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:124
+            .str();
+    emitLayoutRemark(MF, "Stack Layout", "StackLayout", Slot);
+  }
----------------
We usually use identifiers for remark names, so here `StackLayout` instead of `Stack Layout`.


================
Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:178
+
+    emitHeaderRemark(MF);
+
----------------
>From what I can see, you've focused on the `-Rpass` output using diagnostics and tried to emit a pretty-printed version for that on the command line.

We use remarks through their serialized version as well, through `-fsave-optimization-record` which will emit a YAML file that can be used in scripts and other post-processing tools.

I think this should be something in between where it looks user-friendly on the command-line but also easy to post-process.

One way would be to do something similar to [[ https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/MemoryOpRemark.cpp | the memory op remarks ]], which are used here: [[ https://github.com/llvm/llvm-project/blob/main/llvm/test/Transforms/Util/trivial-auto-var-init-call.ll | llvm/test/Transforms/Util/trivial-auto-var-init-call.ll ]].

I could see something where you emit a remark for each slot (+ location), with `ore::NV` used for each piece of information that is useful, something like:

```
ORE << MachineOptimizationRemarkAnalysis(...) << "Stack slot: offset: " << ore::NV("Offset", D.offset)
                                                                                    << "type: " << ore::NV("Type", type)
[...]
```

and could generate something like:

```
 --- !Analysis
 Pass:            stack-frame-layout
 Name:            StackSlot
 Function:        stackSizeWarning
 Args:
   - String:          'Stack slot: offset: '
   - Offset:         '[SP-8]'
   - String:          ', type: '
   - Type:            'spill'
   - String:          ', align: '
   - Align:           '16'
   - String:          ', size: '
   - Align:           '8'
 ...
```

which would look like this on the command line:

```
remark: Stack slot: offset: [SP-8], type: spill, align: 16, size 8
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488