Clean up handling of ARM IR instruction "mov".

src/IceInstARM32.cpp

 //===- subzero/src/IceInstARM32.cpp - ARM32 instruction implementation ----===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 997 matching lines @@
 template <> const char *InstARM32Tst::Opcode = "tst";
 
 void InstARM32::dump(const Cfg *Func) const {
   if (!BuildDefs::dump())
     return;
   Ostream &Str = Func->getContext()->getStrDump();
   Str << "[ARM32] ";
   Inst::dump(Func);
 }
 
-void InstARM32Mov::emitMultiDestSingleSource(const Cfg *Func) const {
-  if (!BuildDefs::dump())
-    return;
-  Ostream &Str = Func->getContext()->getStrEmit();
+void InstARM32Mov::emitMultiDestSingleSource(const Cfg *Func,
+                                             const EmitForm Form) const {

[John, 2016/01/16 00:24:13]

I understand what you're doing, but it seems to me that this change makes things
harder to follow. Every method (with the exception of emiUsingForm) is
essentially initializing variables, then going about to do completely different
things for the different output types.

I believe it would be better if, instead of merging the methods, you created
helper functions with common logic (eg, IsVmove), and called them in separate
functions for textual, and binary outputs.

This cl also increases the minimum build, for the compiler will not be able to
DCE the prolog to these functions.

[Karl, 2016/01/19 18:32:39]

Ok. For the moment, I will not try to merge code between the text and binary
forms. Refactoring to match this change.

   Variable *DestLo = getDest();
   Variable *DestHi = getDestHi();
-  auto *Src = llvm::cast<Variable>(getSrc(0));
-
   assert(DestHi->hasReg());
   assert(DestLo->hasReg());
+  auto *Src = llvm::cast<Variable>(getSrc(0));
   assert(llvm::isa<Variable>(Src) && Src->hasReg());
-
-  Str << "\t"
-         "vmov" << getPredicate() << "\t";
-  DestLo->emit(Func);
-  Str << ", ";
-  DestHi->emit(Func);
-  Str << ", ";
-  Src->emit(Func);
+  switch (Form) {
+  case Emit_Text: {
+    if (!BuildDefs::dump())
+      return;
+    Ostream &Str = Func->getContext()->getStrEmit();
+    Str << "\t"
+           "vmov" << getPredicate() << "\t";
+    DestLo->emit(Func);
+    Str << ", ";
+    DestHi->emit(Func);
+    Str << ", ";
+    Src->emit(Func);
+    return;
+  }
+  case Emit_Binary: {
+    auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
+    Asm->setNeedsTextFixup();
+    return;
+  }
+  }
 }
 
-void InstARM32Mov::emitSingleDestMultiSource(const Cfg *Func) const {
-  if (!BuildDefs::dump())
-    return;
-  Ostream &Str = Func->getContext()->getStrEmit();
+void InstARM32Mov::emitSingleDestMultiSource(const Cfg *Func,
+                                             const EmitForm Form) const {
   Variable *Dest = getDest();
   auto *SrcLo = llvm::cast<Variable>(getSrc(0));
   auto *SrcHi = llvm::cast<Variable>(getSrc(1));
 
   assert(SrcHi->hasReg());
   assert(SrcLo->hasReg());
   assert(Dest->hasReg());
   assert(getSrcSize() == 2);
 
-  Str << "\t"
-         "vmov" << getPredicate() << "\t";
-  Dest->emit(Func);
-  Str << ", ";
-  SrcLo->emit(Func);
-  Str << ", ";
-  SrcHi->emit(Func);
+  switch (Form) {
+  case Emit_Text: {
+    if (!BuildDefs::dump())
+      return;
+    Ostream &Str = Func->getContext()->getStrEmit();
+    Str << "\t"
+           "vmov" << getPredicate() << "\t";
+    Dest->emit(Func);
+    Str << ", ";
+    SrcLo->emit(Func);
+    Str << ", ";
+    SrcHi->emit(Func);
+    return;
+  }
+  case Emit_Binary: {
+    auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
+    Asm->setNeedsTextFixup();
+    return;
+  }
+  }
 }
 
 namespace {
 
 bool isVariableWithoutRegister(const Operand *Op) {
   if (const auto *OpV = llvm::dyn_cast<Variable>(Op)) {
     return !OpV->hasReg();
   }
   return false;
 }
 bool isMemoryAccess(Operand *Op) {
   return isVariableWithoutRegister(Op) || llvm::isa<OperandARM32Mem>(Op);
 }
 
 bool isMoveBetweenCoreAndVFPRegisters(Variable *Dest, Operand *Src) {
   const Type DestTy = Dest->getType();
   const Type SrcTy = Src->getType();
   return !isVectorType(DestTy) && !isVectorType(SrcTy) &&
          (isScalarIntegerType(DestTy) == isScalarFloatingType(SrcTy));
 }
 
 } // end of anonymous namespace
 
-void InstARM32Mov::emitSingleDestSingleSource(const Cfg *Func) const {
-  if (!BuildDefs::dump())
-    return;
-  Ostream &Str = Func->getContext()->getStrEmit();
+void InstARM32Mov::emitSingleDestSingleSource(const Cfg *Func,
+                                              const EmitForm Form) const {
   Variable *Dest = getDest();
-
   if (!Dest->hasReg()) {
     llvm::report_fatal_error("mov can't store.");
   }
 
   Operand *Src0 = getSrc(0);
   if (isMemoryAccess(Src0)) {
     llvm::report_fatal_error("mov can't load.");
   }
 
-  Type Ty = Dest->getType();
-  const bool IsVector = isVectorType(Ty);
-  const bool IsScalarFP = isScalarFloatingType(Ty);
+  Type DestTy = Dest->getType();
+  const bool IsVector = isVectorType(DestTy);
+  const bool IsScalarFP = isScalarFloatingType(DestTy);
   const bool CoreVFPMove = isMoveBetweenCoreAndVFPRegisters(Dest, Src0);
   const bool IsVMove = (IsVector || IsScalarFP || CoreVFPMove);
-  const char *Opcode = IsVMove ? "vmov" : "mov";
-  // when vmov{c}'ing, we need to emit a width string. Otherwise, the
-  // assembler might be tempted to assume we want a vector vmov{c}, and that
-  // is disallowed because ARM.
-  const char *WidthString = !CoreVFPMove ? getVecWidthString(Ty) : "";
-  Str << "\t" << Opcode;
   if (IsVMove) {
-    Str << getPredicate() << WidthString;
-  } else {
-    Str << WidthString << getPredicate();
+    switch (Form) {
+    case Emit_Text: {
+      if (!BuildDefs::dump())
+        return;
+      Ostream &Str = Func->getContext()->getStrEmit();
+      Variable *Dest = getDest();
+      Operand *Src0 = getSrc(0);
+      // when vmov{c}'ing, we need to emit a width string. Otherwise, the
+      // assembler might be tempted to assume we want a vector vmov{c}, and
+      // that is disallowed.
+      const bool CoreVFPMove = isMoveBetweenCoreAndVFPRegisters(Dest, Src0);
+      const char *WidthString = !CoreVFPMove ? getVecWidthString(DestTy) : "";
+      Str << "\t"
+          << "vmov" << getPredicate() << WidthString << "\t";
+      Dest->emit(Func);
+      Str << ", ";
+      Src0->emit(Func);
+      return;
+    }
+    case Emit_Binary: {
+      auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
+      if (!llvm::isa<Variable>(Src0))
+        // TODO(kschimpf) Handle moving contants into registers.
+        return Asm->setNeedsTextFixup();
+
+      // Move register to register.
+      switch (DestTy) {
+      default:
+        // TODO(kschimpf): Fill this out more.
+        return Asm->setNeedsTextFixup();
+      case IceType_f32:
+        Type Src0Ty = Src0->getType();
+        switch (Src0Ty) {
+        default:
+          // TODO(kschimpf): Fill this out more.
+          return Asm->setNeedsTextFixup();
+        case IceType_i32:
+          return Asm->vmovsr(Dest, Src0, getPredicate());
+        }
+      }
+    }
+    }
   }
-  Str << "\t";
-  Dest->emit(Func);
-  Str << ", ";
-  Src0->emit(Func);
+
+  switch (Form) {
+  case Emit_Text: {
+    Ostream &Str = Func->getContext()->getStrEmit();
+    Str << "\t"
+        << "mov" << getPredicate() << "\t";
+    getDest()->emit(Func);
+    Str << ", ";
+    getSrc(0)->emit(Func);
+    return;
+  }
+  case Emit_Binary: {
+    auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
+    Asm->mov(getDest(), getSrc(0), getPredicate());
+    return;
+  }
+  }
 }
 
-void InstARM32Mov::emitIASSingleDestSingleSource(const Cfg *Func) const {
-  auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
-  Variable *Dest = getDest();
-  Operand *Src0 = getSrc(0);
-
-  if (!Dest->hasReg()) {
-    llvm::report_fatal_error("mov can't store.");
+void InstARM32Mov::emitUsingForm(const Cfg *Func, const EmitForm Form) const {
+  assert(!(isMultiDest() && isMultiSource()) && "Invalid vmov type.");
+  if (isMultiDest()) {
+    emitMultiDestSingleSource(Func, Form);
+    return;
   }
 
-  if (isMemoryAccess(Src0)) {
-    llvm::report_fatal_error("mov can't load.");
+  if (isMultiSource()) {
+    emitSingleDestMultiSource(Func, Form);
+    return;
   }
-
-  const Type DestTy = Dest->getType();
-  const bool DestIsVector = isVectorType(DestTy);
-  const bool DestIsScalarFP = isScalarFloatingType(DestTy);
-  const bool CoreVFPMove = isMoveBetweenCoreAndVFPRegisters(Dest, Src0);
-  if (DestIsVector || DestIsScalarFP || CoreVFPMove)
-    return Asm->setNeedsTextFixup();
-  return Asm->mov(Dest, Src0, getPredicate());
+  emitSingleDestSingleSource(Func, Form);
 }
 
 void InstARM32Mov::emit(const Cfg *Func) const {
   if (!BuildDefs::dump())
     return;
-  assert(!(isMultiDest() && isMultiSource()) && "Invalid vmov type.");
-  if (isMultiDest()) {
-    emitMultiDestSingleSource(Func);
-    return;
-  }
-
-  if (isMultiSource()) {
-    emitSingleDestMultiSource(Func);
-    return;
-  }
-
-  emitSingleDestSingleSource(Func);
+  emitUsingForm(Func, Emit_Text);
 }
 
 void InstARM32Mov::emitIAS(const Cfg *Func) const {
-  assert(!(isMultiDest() && isMultiSource()) && "Invalid vmov type.");
+  emitUsingForm(Func, Emit_Binary);
   auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
-  if (!(isMultiDest() || isMultiSource()))
-    // Must be single source/dest.
-    emitIASSingleDestSingleSource(Func);
-  else
-    Asm->setNeedsTextFixup();
   if (Asm->needsTextFixup())
     emitUsingTextFixup(Func);
 }
 
 void InstARM32Mov::dump(const Cfg *Func) const {
   if (!BuildDefs::dump())
     return;
   assert(getSrcSize() == 1 || getSrcSize() == 2);
   Ostream &Str = Func->getContext()->getStrDump();
   Variable *Dest = getDest();
@@ skipping 1040 matching lines @@
 
 template class InstARM32FourAddrGPR<InstARM32::Mla>;
 template class InstARM32FourAddrGPR<InstARM32::Mls>;
 
 template class InstARM32CmpLike<InstARM32::Cmn>;
 template class InstARM32CmpLike<InstARM32::Cmp>;
 template class InstARM32CmpLike<InstARM32::Tst>;
 
 } // end of namespace ARM32
 } // end of namespace Ice