[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