[PATCH] D105064: [llvm-mca] Fix JSON output

[Wolfgang Pieb via Phabricator via llvm-commits]

wolfgangp added inline comments.


================
Comment at: llvm/test/tools/llvm-mca/JSON/X86/views.s:57-79
+# CHECK-NEXT:   "Instructions and CPU resources": {
+# CHECK-NEXT:     "Instructions": [
+# CHECK-NEXT:       "addl\t%eax, %eax",
+# CHECK-NEXT:       "addl\t%ebx, %ebx",
+# CHECK-NEXT:       "addl\t%ecx, %ecx",
+# CHECK-NEXT:       "addl\t%edx, %edx"
+# CHECK-NEXT:     ],
----------------
andreadb wrote:
> Why do we nest the name/value pairs for "Resources" and "Instructions" under "Instructions and CPU resources"?
> Those should be at the top, and ideally they should not be nested under anything else.
> 
> 
I had put that in originally, so the instruction and resource list is treated like a view in its own right (not that it is one, just from a design point of view). Perhaps it needs to be redesigned?


================
Comment at: llvm/tools/llvm-mca/Views/View.h:19-22
 #include "llvm/MCA/HWEventListener.h"
-#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/JSON.h"
+#include "llvm/Support/raw_ostream.h"
 
----------------
markoshorro wrote:
> andreadb wrote:
> > Is this because of clang-format? Otherwise please revert.
> Yes, this was because of clang-format, should I revert it anyways?
Yeah, generally we don't want to mix functional and (independent) formatting changes in the same patch. You're welcome to submit a second patch with just formatting changes, however.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105064