Make opt-viewer more usable by general developers - part 1/N

[OfekShilon]

Summary

This is a submission of the OptView2 project to be considered upstream.

The suggested changes mostly try to improve SnR (for general devs, as opposed to compiler authors):

1. Exclude system headers by default,
2. Collect only optimization failures by default,
   1. Create option to split processing into subfolders ('--split-top-folders') to enable processing of large projects that typically break otherwise.
3. Trim repeated remark annotations in source - keep only 5 per line.
4. Enable filtering by remark name/text, preferrably via config file (but possible via command line too). Check config.yaml for some examples.
5. A complete rewrite of the index html, making the table sortable and resizable, display a single entry per file/line, use abridged func names, and display remark name instead of pass.

Except the rewrite of index.html and culling of duplicities, all changes can be reverted by command line options.
I uploaded some results of runs over medium/large projects, to get an impression of it:

1. cpython
2. opencv
3. mujoco

[OfekShilon]

(1) As far as I can see the debian build failure has nothing to do with this diff:

...
unsupported_code_directive	1
unsupported_compiler_option	0
unsupported_source_language	0
🚨 Error: The command exited with status 1

Perhaps the build can be invoked again?

(2) Forgot to mention explicitly: this submission includes a sample cpp project. I hope to extend it later with some guidance on how to use attributes (restrict, const, pure, noescape) to solve common opt remarks. Where might be a good place to add that?

[fhahn]

Thank you very much for sharing those improvements!

The improvements in both UI and remark handling look very desirable in general. Some of the changes seem unrelated, would it be possible to split the patch up in multiple separate patches to make them easier to review individually? E.g. move the UI changes in one or more separate patches, individual patches for the new options, the example and so on.

llvm/tools/opt-viewer/cpp_optimization_example/main.cc

2

Adding an example is great! Ig might also be good to use it as an end-to-end-test, although this may a bit more complicated, as it would require Clang to also be built.

It would also be good to mention this example in some of our documentation.

llvm/tools/opt-viewer/requirements.txt

4

Is this needed or not?

[OfekShilon]

@fhahn You're right of course. I trimmed this patch to include just the added example and the two-line python requirements file. Hopefully this would be an easy approval and I'll add more gradual patches to be reviewed.

Regarding documentation, 3 directions come to mind:

1. Since llvm is on now github, a README.md file can be used. I have a decent starting point ready here (after renaming from "OptView2" of course) and this might be the easiest.
2. Alternatively, this content can be spilled into ./llvm/docs/Remarks.rst - extending the existing opt-viewer section
3. Finally I can create a separate ./llvm/docs/OptViewer.rst and link to it from Remarks.rst.

This will probably be the last patch I submit anyway. What do you think?

[OfekShilon]

Added cpp-example and python requirements files (and not .gitignore) to the CMakeList installation.

[fhahn]

In D131518#3721299, @OfekShilon wrote:

@fhahn You're right of course. I trimmed this patch to include just the added example and the two-line python requirements file. Hopefully this would be an easy approval and I'll add more gradual patches to be reviewed.

Regarding documentation, 3 directions come to mind:

1. Since llvm is on now github, a README.md file can be used. I have a decent starting point ready here (after renaming from "OptView2" of course) and this might be the easiest.
2. Alternatively, this content can be spilled into ./llvm/docs/Remarks.rst - extending the existing opt-viewer section
3. Finally I can create a separate ./llvm/docs/OptViewer.rst and link to it from Remarks.rst.

This will probably be the last patch I submit anyway. What do you think?

Sounds good to me. I think we should probably have a readme and it might be worth to split off the existing opt-viewer section and add a separate .rst for it.

llvm/tools/opt-viewer/cpp_optimization_example/Makefile

1

Not sure if there's an easy way to do it, but if possible it would be good to default to custom build of clang, if that's enabled. Not sure fi that's easily possible though.

llvm/tools/opt-viewer/cpp_optimization_example/main.cc

1

It would probably be good to add the llvm license header here

5

It would probably be good to format this with clang-format and the LLVM code style.

14

nit: stray new line?

llvm/tools/opt-viewer/cpp_optimization_example/run_optview.sh

2

Should this also create a virtual-env and install dependencies?

[fhahn]

I think the pre-commit tests are failing because llvm/tools/opt-viewer/cpp_optimization_example/main.cc doesn't meet the LLVM style guide and needs re-formatting with clang-format. In the LLVM codebase we also usually use .cpp instead of .cc as file extension.

[OfekShilon]

I apologize - I dropped this for too long and am having a hard time resurrecting it now. I'm closing this and will start again in a fresh review request in the near days.

llvm/tools/opt-viewer/cpp_optimization_example/main.cc

5

@fhahn How do I do that?

14

fixed

llvm/tools/opt-viewer/requirements.txt

4

Needed, in parts not yet included in this first patch

[fhahn]

I apologize - I dropped this for too long and am having a hard time resurrecting it now. I'm closing this and will start again in a fresh review request in the near days.

No worries, thanks!

llvm/tools/opt-viewer/CMakeLists.txt

   "opt-viewer.py"
   "optpmap.py"
   "optrecord.py"
-  "style.css")
+  "style.css"
+  "requirements.txt"
+  "cpp_optimization_example/main.cc"
+  "cpp_optimization_example/Makefile"
+  "cpp_optimization_example/run_optview.sh")
 
 foreach (file ${files})
   install(PROGRAMS ${file}

llvm/tools/opt-viewer/cpp_optimization_example/.gitignore

+example
+*.o
+yaml_optimization_remarks
+.depend
+.vscode
+html_output

llvm/tools/opt-viewer/cpp_optimization_example/Makefile

+CC=clang++
+CXX=clang++
+RM=rm -f
+MKDIR_P=mkdir -p
+OBJS_DIR=objs
+DEPENDS_FILE=${OBJS_DIR}/.depend
+YAML_OUTPUT_DIR=yaml_optimization_remarks
+OPTIMIZATION_RECORD_FLAGS=-fsave-optimization-record -foptimization-record-file=./${YAML_OUTPUT_DIR}/$(patsubst %.o,%.opt.yaml,$(notdir $@))
+CPPFLAGS=-O3 -std=c++17
+LDFLAGS=
+LDLIBS=
+BINARY_NAME=example
+
+SRCS=$(wildcard *.cc)
+OBJS=$(addprefix $(OBJS_DIR)/,$(subst .cc,.o,$(SRCS)))
+
+.PHONY: output_folder
+
+all: output_folder ${BINARY_NAME}
+
+directories: ${YAML_OUTPUT_DIR} ${OBJS_DIR}
+
+${YAML_OUTPUT_DIR} ${OBJS_DIR}:
+  ${MKDIR_P} $@
+
+${BINARY_NAME}: $(OBJS)
+  $(CXX) $(LDFLAGS) -o $(BINARY_NAME) $(OBJS) $(LDLIBS)
+
+${OBJS_DIR}/%.o : %.cc
+  $(CXX) -c $(CFLAGS) $(CPPFLAGS) $(OPTIMIZATION_RECORD_FLAGS) $< -o $@
+
+depend: $(DEPENDS_FILE)
+
+$(DEPENDS_FILE): $(SRCS)
+  $(RM) $(DEPENDS_FILE)
+  $(MKDIR_P) ${YAML_OUTPUT_DIR} ${OBJS_DIR}
+  $(CXX) $(CPPFLAGS) -MM $^ >> $(DEPENDS_FILE)
+
+include $(DEPENDS_FILE)
+
+clean:
+  $(RM) -r ${YAML_OUTPUT_DIR} ${OBJS_DIR}
+

llvm/tools/opt-viewer/cpp_optimization_example/main.cc

+#include <iostream>
+#include <vector>
+
+// Based on Roi Barkan - "Argument passing, core guidelines and concepts" - https://www.youtube.com/watch?v=uylFACqcWYI
+void scale_down(std::vector<double>& v, const double& a) {
+    for (auto& item : v) {
+        item /= a;
+    }
+}
+
+void scale_down_example() {
+    std::vector<double> v {2, 1, 2, 3, 4};
+    scale_down(v, v[0]);
+
+}
+
+int main() {
+    scale_down_example();
+
+    return 0;
+}

llvm/tools/opt-viewer/cpp_optimization_example/run_optview.sh

+#!/bin/bash
+set -euo pipefail
+cd "$(dirname "$0")" || exit 1
+
+echo "Running make..."
+make
+
+echo "Running opt-viewer..."
+../opt-viewer.py --open-browser --output-dir ./html_output --source-dir ./ ./yaml_optimization_remarks

llvm/tools/opt-viewer/requirements.txt

+PyYAML>=3.12
+Pygments>=2.2.0