[PATCH] D60851: [llvm-exegesis] Insert IACA markers

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

> The goal of the project is to be able to compare these latencies with the data that are currently in LLVM, and with other simulators and benchmarks.

Okay, that is the goal of the //changes//.
But what's the actual real endgoal?
Generating llvm schedule data from IACA data, not measurements?
I'm sorry if that question makes no sense, i may be unaware of some previous discussions about this.

Relevant: https://bugs.llvm.org/show_bug.cgi?id=38437

In D60851#1475928 <https://reviews.llvm.org/D60851#1475928>, @oontvoo wrote:

> In D60851#1471981 <https://reviews.llvm.org/D60851#1471981>, @lebedev.ri wrote:
>
> > Please add description to differential.
> >  What is the goal? What is this solving?
>
>
> I've added additional details on the project. PTAL. Thanks!


Other comments weren't addressed though.
Also, please always upload all patches with full context (`-U99999`)



================
Comment at: llvm/tools/llvm-exegesis/lib/InlineAsmMCExpr.h:2
+#ifndef THIRD_PARTY_LLVM_LLVM_TOOLS_LLVM_EXEGESIS_INLINEASMMCEXPR_H_
+#define THIRD_PARTY_LLVM_LLVM_TOOLS_LLVM_EXEGESIS_INLINEASMMCEXPR_H_
+
----------------
Missing starting comment, wrong header guard


================
Comment at: llvm/unittests/tools/llvm-exegesis/BenchmarkRunnerTest.cpp:13-20
+#include "third_party/absl/flags/flag.h"
+#include "third_party/llvm/llvm/include/llvm/MC/MCExpr.h"
+#include "third_party/llvm/llvm/include/llvm/MC/MCInst.h"
+#include "third_party/llvm/llvm/include/llvm/MC/MCInstBuilder.h"
+#include "third_party/llvm/llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.h"
+#include "third_party/llvm/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h"
+#include "third_party/llvm/llvm/tools/llvm-exegesis/lib/InlineAsmMCExpr.h"
----------------
All these look wrong


================
Comment at: llvm/unittests/tools/llvm-exegesis/BenchmarkRunnerTest.cpp:59
+
+    Code.LiveIns.push_back(llvm::X86::EAX);
+    Code.LiveIns.push_back(llvm::X86::EBX);
----------------
This is x86-specific, needs to go into `x86/` dir


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60851