[PATCH] D44519: Add llvm-exegesis tool.
Simon Pilgrim via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 30 06:43:40 PDT 2018
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
More information about the llvm-commits
mailing list