[PATCH] D48691: [llvm-mca] Add a HardwareUnit and Context classes.

Clement Courbet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 2 04:33:36 PDT 2018


courbet added a comment.

Great, only a few nits.



================
Comment at: tools/llvm-mca/Context.cpp:30
+
+// Construct a simple pipeline for simulating an out-of-order backend.
+std::unique_ptr<Pipeline> Context::createDefaultPipeline(
----------------
The comment is already on the declaration. I think you can remove it here.


================
Comment at: tools/llvm-mca/Context.h:12
+/// This file defines a class for holding ownership of various simulated
+/// hardware components.  A Context also provides utility routines that make
+/// use of its simulated hardware components.
----------------
I don;t see any such "utility routines" for now. Remove ?


================
Comment at: tools/llvm-mca/HardwareUnit.h:25
+  HardwareUnit &operator=(const HardwareUnit &H) = delete;
+  virtual ~HardwareUnit() = default;
+};
----------------
[nit] Let's make this the class anchor.


https://reviews.llvm.org/D48691





More information about the llvm-commits mailing list