[PATCH] D136677: [AMDGPU] Small cleanups in wait counter code

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp:307-310
     if (T == EXP_CNT) {
-      unsigned UB = ScoreUBs[T] - getWaitCountMax(EXP_CNT);
-      if (ScoreLBs[T] < UB && UB < ScoreUBs[T])
-        ScoreLBs[T] = UB;
+      unsigned UB = ScoreUBs[EXP_CNT] - getWaitCountMax(EXP_CNT);
+      if (ScoreLBs[EXP_CNT] < UB && UB < ScoreUBs[EXP_CNT])
+        ScoreLBs[EXP_CNT] = UB;
----------------
foad wrote:
> scott.linder wrote:
> > Some more thoughts while reviewing (although not applicable to your changes directly):
> > 
> >   - Why is `EXP_CNT` special here?
> >   - Is there a particular reason this relies on unsigned integer underflow? Does this map better onto the conceptual model of the scores/counts?
> > 
> > For either/both can anyone devise a comment to clarify? I suggested what I believe to be an equivalent version which I find easier to understand, but I am likely missing something.
> I don't know why EXP_CNT is handled specially, but I agree your version of the code is simpler and easier to understand.
I did look into this a bit more, and it's to do with whether we still need to emit any wait for a counter that has overflowed. See D70418. But I still don't know why EXP_CNT is treated specially.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136677