[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