[PATCH] D99339: [MCA] Support carry-over instructions for in-order processors

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added inline comments.


================
Comment at: llvm/lib/MCA/Stages/InOrderIssueStage.cpp:339-340
+    if (CarryOver <= Bandwidth) {
+      LLVM_DEBUG(dbgs() << "[E] Carry over (complete) #" << CarriedOver
+                        << " \n");
+
----------------
asavonic wrote:
> andreadb wrote:
> > Nit:  The `[E]` notation should only be used for real events.
> > 
> > I noticed this in previous patches too, but I didn't say anything because it was not so important. Also, those debug comments are very useful in general.
> > 
> > However, it would be nicer if we could use a different prefix for these new remarks; messages like this one are not strictly related to hardware events. It would be nicer if we consistently use a different char for it (maybe N - as in "Note", or R for "Remark"?) instead of `E`, which is supposed to be used as prefix for event-related debug messages.
> "N" as in Note sounds good.
> I'll update other log messages in a separate NFC patch.
Sounds good! Thanks!

The patch LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99339