[PATCH] D96898: [NFC][Regalloc] Share the VirtRegAuxInfo object with LiveRangeEdit

Mircea Trofin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 18 18:55:28 PST 2021


mtrofin marked 2 inline comments as done.
mtrofin added inline comments.


================
Comment at: llvm/lib/CodeGen/RegAllocPBQP.cpp:806-809
+  // FIXME: we create DefaultVRAI here to match existing behavior pre-passing
+  // the VRAI through the spiller to the live range editor. However, it probably
+  // makes more sense to pass the PBQP VRAI. The existing behavior had
+  // LiveRangeEdit make its own VirtRegAuxInfo object.
----------------
wmi wrote:
> Did you see any problem by using PBQP VRAI?
Tests passed fine, however, I'd prefer decoupling that fix from this change: should there be a regression in the field for the users of PBQP, it'd be revertible without affecting this change.


================
Comment at: llvm/lib/CodeGen/SplitKit.h:456-457
+  SplitEditor(SplitAnalysis &SA, AAResults &AA, LiveIntervals &LIS,
+              VirtRegMap &VRM, MachineDominatorTree &MDT,
+              MachineBlockFrequencyInfo &MBFI, VirtRegAuxInfo &VRAI);
 
----------------
wmi wrote:
> Unnecessary to pass LIS, VRM and MBFI since they are all included in VRAI?
They aren't public though. We can change the API, but I think that's a separate patch, wdyt?

Also, it'd be debatable if that doesn't couple SplitEditor to the fact that VRAI has such members. But we can discuss that there.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96898/new/

https://reviews.llvm.org/D96898



More information about the llvm-commits mailing list