[PATCH] Change the policy of ZERO_EXTEND/SIGN_EXTEND for SelectionDAG builder and legalization

Tim Northover t.p.northover at gmail.com
Tue Aug 19 04:29:43 PDT 2014


Hi Jiangning,

I think this is mostly fine. As usual, just a few nits.

Cheers.

Tim.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:875
@@ +874,3 @@
+    if (OpL->getOpcode() == ISD::AssertSext &&
+        NewLHS->getOpcode() == ISD::TRUNCATE &&
+        OpR->getOpcode() == ISD::AssertSext &&
----------------
I think this can be generalized slightly. It looks like the check against TRUNCATE is really just guarding against us deciding to apply sign extension to (assertsext LHS, i16) when we're actually emitting an i8 setcc.

I'd suggest something like
    if (OpL->getOpcode() == ISD::AssertSext &&
        cast<VTSDNode>(OpL->getOperand(1))->getVT() == NewLHS.getValueType() &&
        [...]

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:878-879
@@ +877,4 @@
+        NewRHS->getOpcode() == ISD::TRUNCATE) {
+      NewLHS = SExtPromotedInteger(NewLHS);
+      NewRHS = SExtPromotedInteger(NewRHS);
+    } else {
----------------
Since we know the node is AssertSext at this point we don't need to do it again. You can just set NewLHS to OpL and NewRHS to OpR.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:778-779
@@ +777,4 @@
+  unsigned int NumOfSigned = 0, NumOfUnsigned = 0;
+  for (Value::const_user_iterator I = V->user_begin(), E = V->user_end();
+       I != E; ++I) {
+    const User *UI = *I;
----------------
Isn't there a "V->users()" adapter so you can use a range-based loop here?

It might be neater to split this logic into a separate static function too ("getPreferredExtendForValue" or something). I can see the body of that loop growing over time (with sdiv and udiv being the obvious extensions, but probably not the only ones).

http://reviews.llvm.org/D4967






More information about the llvm-commits mailing list