SplitRes_SELECT improvement

[Relph, Richard]

On Mar 12, 2013, at 9:25 AM, Duncan Sands <baldrick at free.fr<mailto:baldrick at free.fr>> wrote:

Hi Richard,

On 12/03/13 16:00, Relph, Richard wrote:
DAGTypeLegalizer::SplitRes_SELECT() currently does an assert if the condition is not of type MVT::i1. This patch uses the existing type of the condition to generate the type for the split conditions instead of a fixed i1.
This is an issue for us when trying to legalize a 8x or 16x vector to a 4x type… it isn't the case that we always have an MVT::i1 condition coming in.

if the select condition is a vector then you should be using a vselect node
rather than a select node.  So I think this patch is wrong and the real problem
is that the wrong select node type is being generated somewhere.

That makes sense… except you'll note that the existing code seems to expect a vector condition, which (as I understand it) is the difference between SELECT and VSELECT... why would it do that for a SELECT?
Can end up in this code for VSELECT, despite the name of the function? If not, what's the point of splitting a scalar condition, or testing it's type, or testing that it is a vector?

There was a wrong select node type being generated in visitSIGN_EXTEND(), which was generating a SELECT with a vector (see my separate email on that.) My first attempt at fixing that was to generate a VSELECT if the sign extend was against a vector. With that fix, I had this problem, so it would seem to indicate that the source was 'elsewhere'. However, when I implemented Nadav's solution to the visitSIGN_EXTEND problem (see that same separate email), this problem seems to have disappeared. So color me confused and I'll drop this commit request… though I would appreciate education on the questions above.

Thanks,
Richard




Ciao, Duncan.


Index: lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp (revision 176837)
+++ lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp (working copy)
@@ -490,10 +490,10 @@
   SDValue Cond = N->getOperand(0);
   CL = CH = Cond;
   if (Cond.getValueType().isVector()) {
-    assert(Cond.getValueType().getVectorElementType() == MVT::i1 &&
-           "Condition legalized before result?");
     unsigned NumElements = Cond.getValueType().getVectorNumElements();
-    EVT VCondTy = EVT::getVectorVT(*DAG.getContext(), MVT::i1, NumElements / 2);
+    EVT VCondTy = EVT::getVectorVT(*DAG.getContext(),
+                                   Cond.getValueType().getVectorElementType(),
+                                   NumElements / 2);
     CL = DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, VCondTy, Cond,
                      DAG.getIntPtrConstant(0));
     CH = DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, VCondTy, Cond,



_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits


_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130312/621bd801/attachment.html>