[PATCH] D131930: [mlgo] Add in-development instruction based features for regalloc advisor

[Aiden Grossman via Phabricator via llvm-commits]

aidengrossman added inline comments.


================
Comment at: llvm/lib/CodeGen/MLRegallocEvictAdvisor.cpp:1001
+            LRPosInfo[OverlapCheckCurrentSegment].Pos;
+        Runner->getTensor<int64_t>(FeatureIDs::instructions_mapping -
+                                   1)[OverlapCurrentSegmentPosition *
----------------
mtrofin wrote:
> can't `LRPosInfo[OverlapCheckCurrentSegment].End < CurrentIndex`? So we know the CurrentIndex is below Begin, but it could also be below this segment's end?
> 
> Current instruction: 5
> CurrentSegment: [1,7)
> NextSegment: [2,4)
> NextNextSegment: [2, 8)
> 
`LRPosInfo[OverlapCheckCurrentSegment].End` can be less than `CurrentIndex`, but it shouldn't be missing any instructions. It's not checking if `CurrentIndex` is below begin, but rather if `CurrentIndex` is above the beginning of the current segment, because if the beginning of the current segment being checked is greater than the current index under analysis, it is guaranteed that the segment currently being processed as well as all future segments don't contain the currently being processed index due to the segments being sorted in ascending order by beginning index.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131930