[PATCH] D47718: [Mips] Use UADDO/ADDCARRY instead of ADDC/ADDE

Simon Dardis via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 6 07:45:45 PDT 2018


sdardis added a comment.

The legalization of unsigned i128 addition (https://reviews.llvm.org/rL334094 for the test) looks strange to me in that the determination of the overflow from the first addition is done with setccs. They appear to be originating from the expansion of setcc of an illegal type.

I'm not entirely convinced on performing addsc/addwc sequences over addsc/(multiple addwc) sequences. The overflow bit for the addwc has to be cleared after reading it so that the next addwc in either type of expansion delivers the correct result. At which point you might as well shift that bit into the carry bit in the dsp control register and perform addwc again.

Some additional comments inline.



================
Comment at: lib/Target/Mips/MipsSEISelDAGToDAG.cpp:255
 
+  // Extract the value of the carry flag. The usage of 1 here with
+  // MipsISD::RDDSP corresponds to reading/writing the entire control
----------------
I realised that I got this wrong when I wrote it. This code should be using 0x1f to read the entire register.


================
Comment at: lib/Target/Mips/MipsSEISelDAGToDAG.cpp:295
+    EVT CarryVT = CarryIn.getValueType();
+    SDValue CstAllOnes = CurDAG->getTargetConstant(-1, DL, CarryVT);
+    CarryNode = CurDAG->getMachineNode(Mips::ADDSC, DL, CarryVT, MVT::Glue,
----------------
This has to be materialized into a register as addsc doesn't have an immediate form, use ADDiu, -1 to materialize it.


================
Comment at: lib/Target/Mips/MipsSEISelDAGToDAG.cpp:310
+    // to bit 20 of the dsp control register.
+    ReplaceUses(SDValue(Node, 1), extractCarryFlag(Result, 20, CurDAG));
+  }
----------------
You also need to clear bit 20 in the dsp control register after performing a addwc. On page 40 of the MIPS DSP AFP (MD00374-2B-MIPS32DSP-AFP-03.01.pdf) , it notes that the over/underflow flags are sticky.


Repository:
  rL LLVM

https://reviews.llvm.org/D47718





More information about the llvm-commits mailing list