Subzero. ARM32. Vector lowering. Add.

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 584 matching lines @@
   Asm->udiv(getDest(), getSrc(0), getSrc(1), getPredicate());
   if (Asm->needsTextFixup())
     emitUsingTextFixup(Func);
 }
 
 template <> void InstARM32Vadd::emitIAS(const Cfg *Func) const {
   auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
   const Variable *Dest = getDest();
   switch (Dest->getType()) {
   default:
-    // TODO(kschimpf) Figure if more cases are needed.

[Karl, 2016/01/25 20:46:36]

Please do not remove this TODO, or at least leave some TODO in (i.e. saying that
vector adds not implemented). This will make it easier to find, and know what
needs to be done for the integrated ARM assembler.

-    Asm->setNeedsTextFixup();
+    emitUsingTextFixup(Func);
     break;
   case IceType_f32:
     Asm->vadds(getDest(), getSrc(0), getSrc(1), CondARM32::AL);
+    assert(!Asm->needsTextFixup());

[Karl, 2016/01/25 20:46:36]

Please leave these asserts in. It is checking whether the integrated assembler
did the right thing.

Textual fixup's allows us to test the integrated ARM assembler as a "hybrid"
assembler. That is, if it hasn't been implemented (completely) in the integrated
assembler, the textual assembler (i.e. method emit()) is used.

This assert verifies that the integrated assembler is complete wrt the call to
method vaddd().

Note: the MINIMAL=1 build will automatically remove asserts.

[Eric Holk, 2016/01/25 20:56:19]

Thanks. I added the assert and the TODO back in.

[John, 2016/01/25 20:58:57]

Spoke to Karl. he misread the code. OK to leave as is.

[Eric Holk, 2016/01/25 21:20:33]

Karl's complaint still seems valid. The assert was there to help him tell which
instructions he still needs to add to the assembler, removing the assert makes
it harder to do that. Correct?

[Karl, 2016/01/25 21:28:23]

If you add back the TODO on line 605, it is sufficient to note.

     break;
   case IceType_f64:
     Asm->vaddd(getDest(), getSrc(0), getSrc(1), CondARM32::AL);
+    assert(!Asm->needsTextFixup());
     break;
   }
-  assert(!Asm->needsTextFixup());
 }
 
 template <> void InstARM32Vdiv::emitIAS(const Cfg *Func) const {
   auto *Asm = Func->getAssembler<ARM32::AssemblerARM32>();
   const Variable *Dest = getDest();
   switch (Dest->getType()) {
   default:
     // TODO(kschimpf) Figure if more cases are needed.
     Asm->setNeedsTextFixup();
     break;
@@ skipping 1671 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