[llvm-branch-commits] [llvm] [AMDGPU][Scheduler] Prepare remat stage for rematerializer integration (NFC) (PR #189489)

[Quentin Colombet via llvm-branch-commits]

================
@@ -624,21 +624,20 @@ class PreRARematStage : public GCNSchedStage {
                                 GCNScheduleDAGMILive &DAG) const;
 
     /// Updates the rematerialization's score w.r.t. the current \p RPTargets.
-    /// \p RegionFreq indicates the frequency of each region
-    void update(const BitVector &TargetRegions, ArrayRef<GCNRPTarget> RPTargets,
+    /// \p RegionFreq indicates the frequency of each region. Returns whether
+    /// the new score is null.
+    bool update(const BitVector &TargetRegions, ArrayRef<GCNRPTarget> RPTargets,
----------------
qcolombet wrote:

Just from an API standpoint, the Boolean returned here is confusing because without looking at the comment, at first I thought it meant that the update was successful or not (or an update happened or not (the score didn't change)). It may be a naming issue, but aside from `updateAndIsScoreNull` I don't see a good name and that name tells me we may be doing too much in one function. (BTW the implementation returns that the score is not null so the comment is not aligned here.)

https://github.com/llvm/llvm-project/pull/189489