Subzero. ARM32. Show FP lowering some love.

src/IceInstARM32.h

 //===- subzero/src/IceInstARM32.h - ARM32 machine 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 400 matching lines @@
     Tst,
     Udiv,
     Umull,
     Uxt,
     Vabs,
     Vadd,
     Vcmp,
     Vcvt,
     Vdiv,
     Veor,
+    Vmla,
+    Vmls,
     Vmrs,
     Vmul,
     Vsqrt,
     Vsub
   };
 
   static constexpr size_t InstSize = sizeof(uint32_t);
 
   static const char *getWidthString(Type Ty);
   static const char *getVecWidthString(Type Ty);
   static CondARM32::Cond getOppositeCondition(CondARM32::Cond Cond);
 
   /// Called inside derived methods emit() to communicate that multiple
   /// instructions are being generated. Used by emitIAS() methods to
   /// generate textual fixups for instructions that are not yet
   /// implemented.
   void startNextInst(const Cfg *Func) const;
 
   /// Shared emit routines for common forms of instructions.
   static void emitThreeAddrFP(const char *Opcode, const InstARM32 *Inst,
                               const Cfg *Func);
+  static void emitFourAddrFP(const char *Opcode, const InstARM32 *Inst,
+                             const Cfg *Func);
 
   void dump(const Cfg *Func) const override;
 
   void emitIAS(const Cfg *Func) const override;
 
 protected:
   InstARM32(Cfg *Func, InstKindARM32 Kind, SizeT Maxsrcs, Variable *Dest)
       : InstTarget(Func, static_cast<InstKind>(Kind), Maxsrcs, Dest) {}
 
   static bool isClassof(const Inst *Inst, InstKindARM32 MyKind) {
@@ skipping 249 matching lines @@
       : InstARM32Pred(Func, K, 2, Dest, Predicate), SetFlags(SetFlags) {
     HasSideEffects = SetFlags;
     addSource(Src0);
     addSource(Src1);
   }
 
   static const char *Opcode;
   bool SetFlags;
 };
 
-/// Instructions of the form x := y op z, for vector/FP. We leave these as
-/// unconditional: "ARM deprecates the conditional execution of any instruction
-/// encoding provided by the Advanced SIMD Extension that is not also provided
-/// by the Floating-point (VFP) extension". They do not set flags.
+/// Instructions of the form x := y op z, for vector/FP. we leave these as

[Jim Stichnoth, 2015/12/07 20:58:14]

I think you accidentally lower-cased most of this comment.

[John, 2015/12/08 13:54:24]

Done.

+/// unconditional: "arm deprecates the conditional execution of any instruction
+/// encoding provided by the advanced simd extension that is not also provided
+/// by the floating-point (vfp) extension". they do not set flags.

[Karl, 2015/12/07 19:11:07]

Why are "We" and "They" switched to "we" and "they". Shouldn't they remain
capitalized, since they begin a sentence.

[John, 2015/12/08 13:54:24]

I honestly do not know what happened here... o.O

 template <InstARM32::InstKindARM32 K>
 class InstARM32ThreeAddrFP : public InstARM32 {
   InstARM32ThreeAddrFP() = delete;
   InstARM32ThreeAddrFP(const InstARM32ThreeAddrFP &) = delete;
   InstARM32ThreeAddrFP &operator=(const InstARM32ThreeAddrFP &) = delete;
 
 public:
   /// Create a vector/FP binary-op instruction like vadd, and vsub. Everything
   /// must be a register.
   static InstARM32ThreeAddrFP *create(Cfg *Func, Variable *Dest, Variable *Src0,
@@ skipping 67 matching lines @@
                        CondARM32::Cond Predicate)
       : InstARM32Pred(Func, K, 3, Dest, Predicate) {
     addSource(Src0);
     addSource(Src1);
     addSource(Src2);
   }
 
   static const char *Opcode;
 };
 
+/// Instructions of the form x := x op1 (y op2 z). E.g., multiply accumulate.
+/// we leave these as unconditional: "arm deprecates the conditional execution

[Jim Stichnoth, 2015/12/07 20:58:14]

Capitalization, as above.

[John, 2015/12/08 13:54:24]

Done.

+/// of any instruction encoding provided by the advanced simd extension that is
+/// not also provided by the floating-point (vfp) extension". they do not set

[Karl, 2015/12/07 19:11:07]

Same here for "they" (last sentence).

[John, 2015/12/08 13:54:24]

Done.

+/// flags.
+template <InstARM32::InstKindARM32 K>
+class InstARM32FourAddrFP : public InstARM32 {
+  InstARM32FourAddrFP() = delete;
+  InstARM32FourAddrFP(const InstARM32FourAddrFP &) = delete;
+  InstARM32FourAddrFP &operator=(const InstARM32FourAddrFP &) = delete;
+
+public:
+  // Every operand must be a register.
+  static InstARM32FourAddrFP *create(Cfg *Func, Variable *Dest, Variable *Src0,
+                                     Variable *Src1) {
+    return new (Func->allocate<InstARM32FourAddrFP>())
+        InstARM32FourAddrFP(Func, Dest, Src0, Src1);
+  }
+  void emit(const Cfg *Func) const override {
+    if (!BuildDefs::dump())
+      return;
+    emitFourAddrFP(Opcode, this, Func);
+  }
+  void emitIAS(const Cfg *Func) const override { emitUsingTextFixup(Func); }
+  void dump(const Cfg *Func) const override {
+    if (!BuildDefs::dump())
+      return;
+    Ostream &Str = Func->getContext()->getStrDump();
+    dumpDest(Func);
+    Str << " = ";
+    Str << Opcode << "." << getDest()->getType() << " ";
+    dumpDest(Func);

[Jim Stichnoth, 2015/12/07 20:58:14]

Doesn't this print Dest too many times, since it's already replicated as the
first Src arg?

[John, 2015/12/08 13:54:24]

These instructions' semantics are

x = x [+/-] (a * b)

so I wanted the dump to be

vml[sa].f?? x, x, a, b

This is also consistent with the integer mla/mls instructions (although those
instructions are actual 4 address instructions.)

[Jim Stichnoth, 2015/12/08 19:20:16]

OK, I see.

+    Str << ", ";
+    dumpSources(Func);
+  }
+  static bool classof(const Inst *Inst) { return isClassof(Inst, K); }
+
+private:
+  InstARM32FourAddrFP(Cfg *Func, Variable *Dest, Variable *Src0, Variable *Src1)
+      : InstARM32(Func, K, 3, Dest) {
+    addSource(Dest);
+    addSource(Src0);
+    addSource(Src1);
+  }
+
+  static const char *Opcode;
+};
+
 /// Instructions of the form x cmpop y (setting flags).
 template <InstARM32::InstKindARM32 K>
 class InstARM32CmpLike : public InstARM32Pred {
   InstARM32CmpLike() = delete;
   InstARM32CmpLike(const InstARM32CmpLike &) = delete;
   InstARM32CmpLike &operator=(const InstARM32CmpLike &) = delete;
 
 public:
   static InstARM32CmpLike *create(Cfg *Func, Variable *Src0, Operand *Src1,
                                   CondARM32::Cond Predicate) {
@@ skipping 39 matching lines @@
 using InstARM32Mul = InstARM32ThreeAddrGPR<InstARM32::Mul>;
 using InstARM32Orr = InstARM32ThreeAddrGPR<InstARM32::Orr>;
 using InstARM32Rsb = InstARM32ThreeAddrGPR<InstARM32::Rsb>;
 using InstARM32Rsc = InstARM32ThreeAddrGPR<InstARM32::Rsc>;
 using InstARM32Sbc = InstARM32ThreeAddrGPR<InstARM32::Sbc>;
 using InstARM32Sdiv = InstARM32ThreeAddrGPR<InstARM32::Sdiv>;
 using InstARM32Sub = InstARM32ThreeAddrGPR<InstARM32::Sub>;
 using InstARM32Udiv = InstARM32ThreeAddrGPR<InstARM32::Udiv>;
 using InstARM32Vadd = InstARM32ThreeAddrFP<InstARM32::Vadd>;
 using InstARM32Vdiv = InstARM32ThreeAddrFP<InstARM32::Vdiv>;
+using InstARM32Veor = InstARM32ThreeAddrFP<InstARM32::Veor>;
+using InstARM32Vmla = InstARM32FourAddrFP<InstARM32::Vmla>;
+using InstARM32Vmls = InstARM32FourAddrFP<InstARM32::Vmls>;
 using InstARM32Vmul = InstARM32ThreeAddrFP<InstARM32::Vmul>;
-using InstARM32Veor = InstARM32ThreeAddrFP<InstARM32::Veor>;
 using InstARM32Vsub = InstARM32ThreeAddrFP<InstARM32::Vsub>;
 using InstARM32Ldr = InstARM32LoadBase<InstARM32::Ldr>;
 using InstARM32Ldrex = InstARM32LoadBase<InstARM32::Ldrex>;
 /// MovT leaves the bottom bits alone so dest is also a source. This helps
 /// indicate that a previous MovW setting dest is not dead code.
 using InstARM32Movt = InstARM32TwoAddrGPR<InstARM32::Movt>;
 using InstARM32Movw = InstARM32UnaryopGPR<InstARM32::Movw, false>;
 using InstARM32Clz = InstARM32UnaryopGPR<InstARM32::Clz, false>;
 using InstARM32Mvn = InstARM32UnaryopGPR<InstARM32::Mvn, false>;
 using InstARM32Rbit = InstARM32UnaryopGPR<InstARM32::Rbit, false>;
@@ skipping 124 matching lines @@
   Operand *getCallTarget() const { return getSrc(0); }
   void emit(const Cfg *Func) const override;
   void emitIAS(const Cfg *Func) const override;
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Inst) { return isClassof(Inst, Call); }
 
 private:
   InstARM32Call(Cfg *Func, Variable *Dest, Operand *CallTarget);
 };
 
-/// Pop into a list of GPRs. Technically this can be predicated, but we don't
-/// need that functionality.
+/// Pops a list of registers. It may be a list of GPRs, or a list of VFP "s"
+/// regs, but not both. in any case, the list must be sorted.

[Jim Stichnoth, 2015/12/07 20:58:14]

In (capitalized)

[John, 2015/12/08 13:54:24]

Done.

 class InstARM32Pop : public InstARM32 {
   InstARM32Pop() = delete;
   InstARM32Pop(const InstARM32Pop &) = delete;
   InstARM32Pop &operator=(const InstARM32Pop &) = delete;
 
 public:
   static InstARM32Pop *create(Cfg *Func, const VarList &Dests) {
     return new (Func->allocate<InstARM32Pop>()) InstARM32Pop(Func, Dests);
   }
   void emit(const Cfg *Func) const override;
   void emitIAS(const Cfg *Func) const override;
   void dump(const Cfg *Func) const override;
   static bool classof(const Inst *Inst) { return isClassof(Inst, Pop); }
 
 private:
   InstARM32Pop(Cfg *Func, const VarList &Dests);
 
   VarList Dests;
 };
 
-/// Push a list of GPRs. Technically this can be predicated, but we don't need
-/// that functionality.
+/// Pushs a list of registers. Just like Pop (see above) the list may be of

[Jim Stichnoth, 2015/12/07 20:58:14]

Pushes

[Jim Stichnoth, 2015/12/07 20:58:14]

Nit: I would put a comma after "(see above)".

[John, 2015/12/08 13:54:24]

Done.

[John, 2015/12/08 13:54:24]

Done.

+/// GPRs, or VFP "s" registers, but not both.
 class InstARM32Push : public InstARM32 {
   InstARM32Push() = delete;
   InstARM32Push(const InstARM32Push &) = delete;
   InstARM32Push &operator=(const InstARM32Push &) = delete;
 
 public:
   static InstARM32Push *create(Cfg *Func, const VarList &Srcs) {
     return new (Func->allocate<InstARM32Push>()) InstARM32Push(Func, Srcs);
   }
   void emit(const Cfg *Func) const override;
@@ skipping 279 matching lines @@
 // default implementations. Without this, there is the possibility of ODR
 // violations and link errors.
 
 template <> void InstARM32Ldr::emit(const Cfg *Func) const;
 template <> void InstARM32Movw::emit(const Cfg *Func) const;
 template <> void InstARM32Movt::emit(const Cfg *Func) const;
 
 } // end of namespace Ice
 
 #endif // SUBZERO_SRC_ICEINSTARM32_H