[llvm-commits] [PATCH] Support for non-32bit return values from CC

Rotem, Nadav nadav.rotem at intel.com
Wed Jul 25 13:26:23 PDT 2012


Generally, it looks good.  I had a few questions and comments below. 

Thanks.


+    SDValue SetCC1, SetCC2;
+    if (CCCode != ISD::SETO && CCCode != ISD::SETUO) {
+        SetCC1 = DAG.getSetCC(dl, VT, LHS, RHS, CC1);
+        SetCC2 = DAG.getSetCC(dl, VT, LHS, RHS, CC2);
+    } else {
+        SetCC1 = DAG.getSetCC(dl, VT, LHS, LHS, CC1);
+        SetCC2 = DAG.getSetCC(dl, VT, RHS, RHS, CC2);
     }

Can you explain this part ?

+    ISD::CondCode CCCode = cast<CondCodeSDNode>(CC)->get();
+    EVT Tmp1VT = Tmp1.getValueType();

Maybe this will be a good time to rename Tmp1 to a meaningful name.
	
+      Tmp1 = DAG.getSetCC(dl, TLI.getSetCCResultType(Tmp1VT),
+          Tmp1, Tmp2, CCCode);
+      Tmp1 = DAG.getNode(Tmp1VT.isVector() ? ISD::VSELECT : ISD::SELECT,
+          dl, Node->getValueType(0), Tmp1, Tmp3, Tmp4);

You can use the helper function from the other patch you sent today. 

Also, how do you know that VSELECT is legal and that the condition operand is of the correct type ? It should be okay, since SETCC should be consumed by VSELECT, but I suggest that you add an assertion.

+      Tmp2 = DAG.getConstant(0, Tmp1VT);
+      CC = DAG.getCondCode(ISD::SETNE);
+      Tmp1 = DAG.getNode(ISD::SELECT_CC, dl, Node->getValueType(0), Tmp1, Tmp2,
+          Tmp3, Tmp4, CC);

You need to check if SELECT_CC is supported.  X86 does not support it. 

-    assert(Cond.getValueType().getVectorElementType() == MVT::i1 &&
-           "Condition legalized before result?");

Why did you remove the assertion ? Can the original input mask be a non-i1 vector ?
It looks like this method is used to split vector types, which means that we split based on the result of the node. At this point, the mask operand has to be vxi1.


From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Villmow, Micah
Sent: Wednesday, July 25, 2012 20:38
To: llvm-commits at cs.uiuc.edu
Subject: [llvm-commits] [PATCH] Support for non-32bit return values from CC

It looks like with LLVM 3.1, vselect was added and vsetcc and setcc were combined
, but there are places that still assume the return value of the condition is i1.
However, there is a callback getSetCCResultType, this patch adds in support to
various locations for supporting setCC result types that are NOT i1.

This should apply cleanly to LLVM 3.1 but might not apply to TOT. After I get
feedback that this approach is OK, I'll port it to TOT. This only shows up on
backends that support vectors and don't support i1 as a SetCC result type.

if this approach is not ok, then I need to write lots of custom code in 
my backend to work around this issue, so early feedback is welcome!

Thanks for any feedback,
Micah
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.





More information about the llvm-commits mailing list