[PATCH] D82684: AVR Backend: Add harvard program address space checker pass

[Dylan McKay via Phabricator via llvm-commits]

dylanmckay added a comment.

Patch is looking really good, only suggestions are the adding a test and a couple formatting nitpicks.



================
Comment at: llvm/lib/Target/AVR/HarvardCheck/CMakeLists.txt:1
+add_llvm_library(libHarvardCheck MODULE
+  HarvardCheck.cpp
----------------
Add the prefix `AVR` to the library name, i.e. `libAVRHarvardCheck` to make it clear this dylib is a part of the AVR backend.


================
Comment at: llvm/lib/Target/AVR/HarvardCheck/HarvardCheck.cpp:73
+private:
+  unsigned m_programAddressSpace;
+  unsigned m_errorCount;
----------------
Remove the `m_` prefix from the member field names, as LLVM doesn't use this naming convention.


================
Comment at: llvm/lib/Target/AVR/HarvardCheck/HarvardCheck.cpp:82
+static RegisterPass<HarvardCheck>
+    X("harvard-check", "Verify that all code resides in program memory", false,
+      false);
----------------
Add a test for the new pass, inside `test/CodeGen/AVR` for the new pass. The `RUN:` line can be  `; RUN: opt -load libHarvardCheck.dylib --harvard-check -march=avr 2>&1 < %s | FileCheck %s`, the test can contain functions in the wrong address space, and the `CHECK` lines can verify that the correct error messages are generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82684