[PATCH] D31124: AMDGPU/SI: Add lane tracking to SI Scheduler

Axel Davy via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 22 00:37:05 PDT 2017

axeldavy added inline comments.

Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:1476
+  for (const auto RegLaneMasks : RegWithLaneMask) {
+    SmallVector<LaneBitmask, 8> &LaneBasis =
rampitec wrote:
> Forgot '&'?
indeed. Thanks.

Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:1653
+  for (const auto RegPair : Regs) {
+    for (const auto RegPairRes : getPairsForReg(RegPair.RegUnit,
+                                                RegPair.LaneMask)) {
rampitec wrote:
> Can you please avoid copy here and in function's arguments?
Right, I can get
SIScheduleBlockScheduler::getPairsForRegs(const SmallVector<RegisterMaskPair, 8> &Regs)
  SmallVector<RegisterMaskPair, 8> Result;
  for (const auto &RegPair : Regs) {

But for the line with:
for (const auto RegPairRes : getPairsForReg(RegPair.RegUnit,

Do you mean just use &RegPairRes ?
Daniel Berlin suggested to use std::transform in combination with a new vector_inserter to replace the two loops.
That wouldn't prevent the fact there is in all cases a copy from getPairsForReg results into the result tab of getPairsForRegs.

I could probably solve the issue by adding a new getPairsForReg function that takes the Result array to append to, and using that helper with the two functions. Do you think that would be a good solution ?

Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:1728
     dbgs() << "\nCurrent Live:\n";
-    for (unsigned Reg : LiveRegs)
-      dbgs() << PrintVRegOrUnit(Reg, DAG->getTRI()) << ' ';
+    for (const auto RegPair : LiveRegs) {
+      dbgs() << PrintVRegOrUnit(RegPair.first, DAG->getTRI());
rampitec wrote:
> auto& (and in other places too)?



More information about the llvm-commits mailing list