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

Matt Arsenault via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 9 16:46:20 PST 2016


arsenm added a comment.

In http://reviews.llvm.org/D17898#371395, @jvesely wrote:

> In http://reviews.llvm.org/D17898#368335, @arsenm wrote:
>
> > What is emitting the illegal min/max?
>
>
> the culprit is SelectionDAGBuilder::visitSelect(const User &I), in lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2598.
>
>   it transforms specifc single use comparisons into min/max.
>   
>
> It does not check legality of min/max operations. even for the operations it does check, it considers type after type legalization (so the check would pass on r600). The comment on line 2610 indicates that it intentionally relies on type legalizer handling illegal ops/types.


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


================
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 =
----------------
Why is the code so different for each of these? I would expect the same function with the CondCode as an operand


Repository:
  rL LLVM

http://reviews.llvm.org/D17898





More information about the llvm-commits mailing list