Currently we don't generate the scheduler comments for instructions inside inline asm. The patch fixes that and address PR35162.
Details
Details
Diff Detail
Diff Detail
- Repository
- rL LLVM
Event Timeline
Comment Actions
Overall, the patch looks good to me.
I made some minor comments below.
| include/llvm/MC/MCParser/MCAsmParser.h | ||
|---|---|---|
| 131 ↗ | (On Diff #121870) | Comments should end with a period. I noticed the same mistake in AsmPrinter.h. | 
| 135 ↗ | (On Diff #121870) | Unrelated to your code review. However, this comment should be associated to field 'HadError' at line 130. I think this should be fixed. | 
| 169 ↗ | (On Diff #121870) | I suggest to rename enablePrintSchedInfo() to shouldPrintSchedInfo(). | 
| lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp | ||
| 198 ↗ | (On Diff #121870) | Add a /* unused */ for the bool param. | 
| 1046–1047 ↗ | (On Diff #121870) | I suggest to change EnableSchedInfo to just PrintSchedInfo (or PrintSchedInfoEnabled). | 
| 1050–1052 ↗ | (On Diff #121870) | Same. | 
| lib/Target/X86/AsmParser/X86AsmInstrumentation.h | ||
| 45–46 ↗ | (On Diff #121870) | I suggest to change EnableSchedInfo to just PrintSchedInfo (or PrintSchedInfoEnabled). | 
| 58–59 ↗ | (On Diff #121870) | Same. | 
| lib/Target/X86/AsmParser/X86AsmParser.cpp | ||
| 2704 ↗ | (On Diff #121870) | I suggest to rename enablePrintSchedInfo() to shouldPrintSchedInfo(). |