[PATCH] D17898: Implement expansion of {s,u}{min,max}

Jan Vesely via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 9 20:12:19 PST 2016


jvesely added a comment.

In http://reviews.llvm.org/D17898#371422, @arsenm wrote:

> OK, this is what I thought I did but it looks like I only did the promotion of integer types handling. I think new tests should be added for a weird sized integer between 32 and 64 that needs to be legalized to i64


Using i56 fails in LegalizeDAG.cpp:893
Assertion `TLI.isTypeLegal(StVT) && "Do not know how to expand this store!"' failed.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:1692-1694
@@ +1691,5 @@
+
+void DAGTypeLegalizer::ExpandIntRes_UMINMAX(SDNode *N,
+                                           SDValue &Lo, SDValue &Hi) {
+  SDLoc dl(N);
+  const llvm::ISD::CondCode Opc =
----------------
arsenm wrote:
> Why is the code so different for each of these? I would expect the same function with the CondCode as an operand
signed version can be used for both as it just reverts back to compare+select.

unsigned version tries to be smarter and recursively applies min/max to low part if high parts are equal (I think this was the intention behind the comment in visitSelect, but the signed version is rather messy).
I can switch the Hi select to another min/max to reduce dependencies.


Repository:
  rL LLVM

http://reviews.llvm.org/D17898





More information about the llvm-commits mailing list