[PATCH] D46243: Move Schedule class to header file for allowing inheritance

[Eugene Zelenko via Phabricator via llvm-commits]

Eugene.Zelenko added inline comments.


================
Comment at: include/llvm/CodeGen/PostRASchedulerList.h:1
+/**/
+
----------------
Please add LLVM header. Just copy from other file and change file name and description.


================
Comment at: include/llvm/CodeGen/PostRASchedulerList.h:20
+
+#ifndef LLVM_CODEGEN_PostRASchedulerList_H
+#define LLVM_CODEGEN_PostRASchedulerList_H
----------------
Please capitalize header guard name. Same in other places. 


================
Comment at: include/llvm/CodeGen/PostRASchedulerList.h:25
+	
+  class PostRAScheduler : public MachineFunctionPass {
+    const TargetInstrInfo *TII;
----------------
Please run clang-format.


================
Comment at: include/llvm/CodeGen/PostRASchedulerList.h:31
+    static char ID;
+    PostRAScheduler() : MachineFunctionPass(ID) {}
+
----------------
Please separate with empty line.


================
Comment at: include/llvm/CodeGen/PostRASchedulerList.h:34
+    void getAnalysisUsage(AnalysisUsage &AU) const override {
+      AU.setPreservesCFG();
+      AU.addRequired<AAResultsWrapperPass>();
----------------
Is this method frequently used? If not, move it to source file to reduce header dependencies. Same for getRequiredProperties().


================
Comment at: include/llvm/CodeGen/PostRASchedulerList.h:57
+  };
+  
+  
----------------
Please remove one of empty line.


================
Comment at: lib/CodeGen/PostRASchedulerList.cpp:77
 
 AntiDepBreaker::~AntiDepBreaker() { }
 
----------------
Please use = default;


Repository:
  rL LLVM

https://reviews.llvm.org/D46243