Subzero: Aggressive LEA

src/IceInstX86BaseImpl.h

 //===- subzero/src/IceInstX86BaseImpl.h - Generic X86 instructions -*- C++ -*=//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 2007 matching lines @@
   Str << "storeq." << this->getSrc(0)->getType() << " ";
   this->getSrc(1)->dump(Func);
   Str << ", ";
   this->getSrc(0)->dump(Func);
 }
 
 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());
   Str << "\t"
          "lea" << this->getWidthString(this->getDest()->getType()) << "\t";
   Operand *Src0 = this->getSrc(0);
   if (const auto *Src0Var = llvm::dyn_cast<Variable>(Src0)) {
     Type Ty = Src0Var->getType();
     // lea on x86-32 doesn't accept mem128 operands, so cast VSrc0 to an
     // acceptable type.
@@ skipping 900 matching lines @@
     return;
   Ostream &Str = Func->getContext()->getStrDump();
   Str << "IACA_END";
 }
 
 } // end of namespace X86NAMESPACE
 
 } // end of namespace Ice
 
 #endif // SUBZERO_SRC_ICEINSTX86BASEIMPL_H