[PATCH] D44519: Add llvm-exegesis tool.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

It'd be interesting to get some opinions from non-X86 teams, whether the current architecture can be made to work for ARM/PPC/MIPS etc.?



================
Comment at: tools/llvm-exegesis/lib/InMemoryAssembler.cpp:54
+    return true;
+  }
+  std::string Banner = std::string("After ") + std::string(P->getPassName());
----------------
RKSimon wrote:
> ```
>   if (!PI->getNormalCtor()) {
>     llvm::errs() << " cannot create pass: " << PI->getPassName() << "\n";
>     return true;
>   }
>   llvm::Pass *P = PI->getNormalCtor()();
> ```
Why not this?
```
  if (!PI->getNormalCtor()) {
    llvm::errs() << " cannot create pass: " << PI->getPassName() << "\n";
    return true;
  }
  llvm::Pass *P = PI->getNormalCtor()();
```


================
Comment at: tools/llvm-exegesis/lib/InMemoryAssembler.cpp:95
+    }
+  }
+  TPC->setInitialized();
----------------
All these braces (for/if) can go.


================
Comment at: tools/llvm-exegesis/lib/InMemoryAssembler.cpp:138
+          Flags |= llvm::RegState::Define;
+        }
+        Builder.addReg(Op.getReg(), Flags);
----------------
Remove braces from single line if()


================
Comment at: tools/llvm-exegesis/lib/InstructionSnippetGenerator.cpp:151
+computeSequentialAssignmentChains(const llvm::MCRegisterInfo &RegInfo,
+                                  const llvm::ArrayRef<Variable> Vars) {
+  using graph::Node;
----------------
Do you need the const on the ArrayRef? I don't think you can change anything - that's what MutableArrayRef is for.


================
Comment at: tools/llvm-exegesis/lib/InstructionSnippetGenerator.cpp:159
+  // Adding variables to the graph.
+  for (size_t I = 0; I < Vars.size(); ++I) {
+    const Variable &Var = Vars[I];
----------------
```
for (size_t I = 0, E = Vars.size(); I < E; ++I) {
```


================
Comment at: tools/llvm-exegesis/lib/InstructionSnippetGenerator.cpp:230
+  // Registers with remaining degrees of freedom are assigned randomly.
+  for (size_t I = 0; I < Vars.size(); ++I) {
+    llvm::MCPhysReg &Reg = Registers[I];
----------------
```
for (size_t I = 0, E = Vars.size(); I < E; ++I) {
```


================
Comment at: tools/llvm-exegesis/lib/LlvmState.h:32
+  const std::string &getCpuName() const { return CpuName; }
+  const std::string &getFeatures() const { return Features; }
+
----------------
Return StringRef instead for these?


================
Comment at: tools/llvm-exegesis/lib/Uops.cpp:101
+    // we never create a RAW hazard that would lead to serialization.
+    for (size_t I = 0; I < Vars.size(); ++I) {
+      llvm::MCPhysReg Reg = Regs[I];
----------------
```
for (size_t I = 0, E = Vars.size(); I < e; ++I) {
```


Repository:
  rL LLVM

https://reviews.llvm.org/D44519