[PATCH] D40960: Fix for bug PR35549 - [X86] Repeated schedule comments

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:745
 /// emitComments - Pretty-print comments for instructions.
-static void emitComments(const MachineInstr &MI, raw_ostream &CommentOS,
+static bool emitComments(const MachineInstr &MI, raw_ostream &CommentOS,
                          AsmPrinter *AP) {
----------------
update the comment to explain what the return value means


================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:794
     CommentOS << " " << MF->getSubtarget().getSchedInfoStr(MI) << "\n";
+    Result = true;
+  }
----------------
just return true here? Then you can remove Result entirely and return false at the end of the function.


================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1045
+          MIP->setFlags (F);
+        }
 
----------------
```
if (isVerbose() && emitComments(MI, OutStreamer->GetCommentOS(), this)) {
```

It'd be much safer to add a MachineInstr::setSchedInfoFlags(bool) style helper and avoid the hard coded constant here.

The whole block needs to be passed through clang-format as well.


================
Comment at: lib/Target/X86/MCTargetDesc/X86BaseInfo.h:63
+    IP_HAS_LOCK = 16,
+    IP_NO_SCHED_INFO = 32
   };
----------------
Add a comment describing the flag as its different from the other flags which are about prefixes.


================
Comment at: lib/Target/X86/X86MCInstLower.cpp:107
+  OutStreamer->EmitInstruction(Inst, getSubtargetInfo(),
+                               EnablePrintSchedInfo & !(Inst.getFlags() & 32));
   SMShadowTracker.count(Inst, getSubtargetInfo(), CodeEmitter.get());
----------------
Again, get rid of this hard coded constant


================
Comment at: lib/Target/X86/X86MCInstLower.cpp:2007
   MCInstLowering.Lower(MI, TmpInst);
+  TmpInst.setFlags(MI->getFlags());
 
----------------
Won't this copy other flags as well? Is that safe?


https://reviews.llvm.org/D40960