Subzero: Aggressive LEA

src/IceInstX86BaseImpl.h

Index: src/IceInstX86BaseImpl.h
diff --git a/src/IceInstX86BaseImpl.h b/src/IceInstX86BaseImpl.h
index cfc36d44a4fb3827d3329385015dc6de47a9631b..45563a7b62b5bc254ca88a47052da14e57fea97d 100644
--- a/src/IceInstX86BaseImpl.h
+++ b/src/IceInstX86BaseImpl.h
@@ -2025,6 +2025,17 @@ template <typename TraitsType>
 void InstImpl<TraitsType>::InstX86Lea::emit(const Cfg *Func) const {
   if (!BuildDefs::dump())
     return;
+
+  auto *MemOp = llvm::dyn_cast<X86OperandMem>(this->getSrc(0));

[Jim Stichnoth, 2016/07/29 14:27:26]

This needs a comment to explain what's going on, which is to revert back to an
add instruction when the lea is actually a 2-address instruction rather than
3-address.

Also, are you missing opportunities?
  eax = lea [eax+edx]
  eax = lea [edx+eax]
  and if the dest register is in the index slot instead of the base slot, though
that probably never actually happens

Finally, any changes like this in an emit() method should have corresponding
changes in the emitIAS() method, otherwise they won't be seen in production.

Related to this, you should probably have a lit test for this.  The normal lit
test would test the emitIAS() path, and the emit() path would get tested when
"make presubmit" runs with FORCEASM=1.

+  if (MemOp && getFlags().getAggressiveLea() &&

[Jim Stichnoth, 2016/07/29 14:27:26]

MemOp != nullptr

[manasijm, 2016/08/01 19:36:21]

Done.

+      MemOp->getBase()->getRegNum() == this->getDest()->getRegNum() &&
+      MemOp->getIndex() == nullptr && MemOp->getShift() == 0) {
+    auto *Add = InstImpl<TraitsType>::InstX86Add::create(
+        const_cast<Cfg *>(Func), this->getDest(), MemOp->getOffset());

[Jim Stichnoth, 2016/07/29 14:27:26]

const_cast - yuck!  This often indicates a design problem, which I think is true
here - all the emit() methods take a const Cfg * arg because we never expected
emit() to modify the CFG.

Can you try using the same allocator trick as in Variable::asType() ?

[manasijm, 2016/08/01 19:36:21]

Done.

[manasijm, 2016/08/01 19:36:21]

Using the allocator 'trick' does not seem to help because the constructor for
InstX86Lea takes a non-const Cfg *

[Jim Stichnoth, 2016/08/01 21:08:41]

well actually, asType() has the same problem with the non-const Cfg, and it gets
around the problem by directly calling the private Variable ctor, which it gets
away with because asType() is a method of the Variable class.

We can't really do the same here with the private ctor, so I guess const_cast
needs to stay for now.

+    Add->emit(Func);
+    return;
+  }
+
   Ostream &Str = Func->getContext()->getStrEmit();
   assert(this->getSrcSize() == 1);
   assert(this->getDest()->hasReg());