Remove possible loop creation in DAGCombiner

[Hal Finkel]

----- Original Message -----
> From: "deadal nix" <deadalnix at gmail.com>
> To: "llvm-commits" <llvm-commits at cs.uiuc.edu>
> Sent: Thursday, September 25, 2014 12:09:07 AM
> Subject: Remove possible loop creation in DAGCombiner
> 
> 
> 
> 
> DAGCombiner can create loop in the DAG when you sext the result of a
> setcc, depending on the target.
> 
> This diff remove the call to DAG.getSExtOrTrunc that create the loop.
> This call used to be necessary for x86 backend, but it doesn't look
> necessary anymore. I've added tests to ensure it doesn't.

-        EVT SelectVT = getSetCCResultType(VT);
-        return DAG.getSelect(DL, VT,
-                             DAG.getSExtOrTrunc(SetCC, DL, SelectVT),
-                             NegOne, DAG.getConstant(0, VT));
-
+        return DAG.getSelect(DL, VT, SetCC,
+                          NegOne, DAG.getConstant(0, VT));
       }

It seems clear to me that creating an SIGN_EXTEND inside of visitSIGN_EXTEND could easily lead to a loop, and we should avoid that. However, getSExtOrTrunc might also have been providing a TRUNC, and if the SIGN_EXTEND really would be needed, we may just want to abort the transformation all together. For one thing, I don't think that it can be different unless VT != N0.getOperand(0).getValueType().

In short, you're proposing to change target-independent code, and I'd be more conservative. Why don't you just disable the transformation when SelectVT.bitsGT(SetCC.getValueType()) is true (which I believe is the condition under which we'd get the SIGN_EXTEND).

 -Hal

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

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory