Subzero. ARM32. Show FP lowering some love.

src/IceInstARM32.h

Index: src/IceInstARM32.h
diff --git a/src/IceInstARM32.h b/src/IceInstARM32.h
index 138fd845c9d6897aa3044ff19ed16e2cf99022d0..f01f5fef13b0165b4580d7979f96d7c42adc73cb 100644
--- a/src/IceInstARM32.h
+++ b/src/IceInstARM32.h
@@ -418,6 +418,8 @@ public:
     Vcvt,
     Vdiv,
     Veor,
+    Vmla,
+    Vmls,
     Vmrs,
     Vmul,
     Vsqrt,
@@ -439,6 +441,8 @@ public:
   /// 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;
 
@@ -708,10 +712,10 @@ private:
   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;
@@ -799,6 +803,54 @@ private:
   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 {
@@ -858,8 +910,10 @@ using InstARM32Sub = InstARM32ThreeAddrGPR<InstARM32::Sub>;
 using InstARM32Udiv = InstARM32ThreeAddrGPR<InstARM32::Udiv>;
 using InstARM32Vadd = InstARM32ThreeAddrFP<InstARM32::Vadd>;
 using InstARM32Vdiv = InstARM32ThreeAddrFP<InstARM32::Vdiv>;
-using InstARM32Vmul = InstARM32ThreeAddrFP<InstARM32::Vmul>;
 using InstARM32Veor = InstARM32ThreeAddrFP<InstARM32::Veor>;
+using InstARM32Vmla = InstARM32FourAddrFP<InstARM32::Vmla>;
+using InstARM32Vmls = InstARM32FourAddrFP<InstARM32::Vmls>;
+using InstARM32Vmul = InstARM32ThreeAddrFP<InstARM32::Vmul>;
 using InstARM32Vsub = InstARM32ThreeAddrFP<InstARM32::Vsub>;
 using InstARM32Ldr = InstARM32LoadBase<InstARM32::Ldr>;
 using InstARM32Ldrex = InstARM32LoadBase<InstARM32::Ldrex>;
@@ -1004,8 +1058,8 @@ 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;
@@ -1026,8 +1080,8 @@ private:
   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;