[PATCH] D46821: Update llvm-exegesis to cover latency through another opcode.

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet planned changes to this revision.
gchatelet marked 6 inline comments as done.
gchatelet added inline comments.


================
Comment at: tools/llvm-exegesis/lib/Assembler.cpp:209
   ExecEngine.reset(
-      llvm::EngineBuilder(std::move(FunctionContext.Module))
+      llvm::EngineBuilder(std::move(AC.Module))
           .setErrorStr(&Error)
----------------
courbet wrote:
> Given that you're moving from AC, I don't think it makes sense to have AC be a member variable here. AC should not be accessible outside of this function.
Yes, that's part of the redesign I intend to do. I added some FIXME in the header file.


================
Comment at: tools/llvm-exegesis/lib/Assembler.cpp:235
+                           llvm::sys::fs::OpenFlags::F_None);
+  if (ErrorCode)
+    llvm::report_fatal_error("Cannot write to file");
----------------
courbet wrote:
> it'd be better if the error was handled by the caller. BTW I don't see who's calling that.
Nobody does for now, it's a leftover from the code I wrote earlier to deal with the process spawning. I'll just remove it for now.


Repository:
  rL LLVM

https://reviews.llvm.org/D46821