[PATCH] D34769: [X86] X86::CMOV to Branch heuristic based optimization

[Zvi Rackover via Phabricator via llvm-commits]

zvi added inline comments.


================
Comment at: lib/Target/X86/X86CmovConversion.cpp:338
+        bool IsCMOV = CmovInstructions.count(&MI);
+        for (auto &MO : MI.operands()) {
+          if (!MO.isReg() || !MO.isUse())
----------------
aaboud wrote:
> zvi wrote:
> > Are these two equivalent?
> > 
> >   for (auto &MO : MI.operands())
> >     if (!MO.isReg() || !MO.isUse())
> >       continue;
> > 
> > and
> > 
> >   for (auto &MO : MI.uses())
> >     if (!MO.isReg())
> >       continue?
> Could be, but to keep the code consistent wit the below loop, I preferred this format, I do not mind change to your suggestion if you think it is better.
Iterating over uses() instead of operand() is more efficient, right?


================
Comment at: lib/Target/X86/X86CmovConversion.cpp:564
+  // destination registers, and the registers that went into the PHI.
+  DenseMap<unsigned, std::pair<unsigned, unsigned>> RegRewriteTable;
+
----------------
Please add comment explaining the data-structure


================
Comment at: lib/Target/X86/X86CmovConversion.cpp:577
+
+    if (RegRewriteTable.find(Op1Reg) != RegRewriteTable.end())
+      Op1Reg = RegRewriteTable[Op1Reg].first;
----------------
  auto I = RegRewriteTable.find(Op1Reg);
  if (I != RegWriteTable.end())
    Op1Reg  = I->second.first


https://reviews.llvm.org/D34769