[PATCH] D30147: AMDGPU/SI: Add new SISched policy to reduce register usage

Valery Pykhtin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 27 07:30:39 PDT 2017


vpykhtin added a comment.

Sorry but I give up reviewing this. Your code looks like after "inline all" pass. Can you refactor common parts into functions with meaningfull names?



================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:1801
+  DenseMap<unsigned, SmallPtrSet<SIScheduleBlock*, 16>> RegisterConsumers;
+  SIBlockInfo() : Dependencies(), ConsumedRegisters(),
+  ProducedConsumedRegisters(), ProducedRegisters(), BlockInRegs(),
----------------
do not need a constructor here, default would do just what you've written


================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:1951
+        // fill any data if the Parent is already scheduled.
+        if (ParentBlockInfoPair != BlockInfos.end()) {
+          SIBlockInfo &ParentBlockInfo = ParentBlockInfoPair->second;
----------------
in such cases its better to revert the condition and do a continue rather than having huge indented {}


Repository:
  rL LLVM

https://reviews.llvm.org/D30147





More information about the llvm-commits mailing list