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

Jiangning Liu liujiangning1 at gmail.com
Tue Aug 19 22:22:02 PDT 2014


Hi Tim,

Thanks for your comments! And I updated the code following your comments.

Thanks again!
-Jiangning

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:875
@@ +874,3 @@
+    if (OpL->getOpcode() == ISD::AssertSext &&
+        NewLHS->getOpcode() == ISD::TRUNCATE &&
+        OpR->getOpcode() == ISD::AssertSext &&
----------------
Tim Northover wrote:
> 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() &&
>         [...]
OK. I changed the code to be like this. But I think the only case is NewLHS and NEWRHS are TRUNCATE, so I added assertions inside the if body. Is there any other case except TRUNCATE?

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:878-879
@@ +877,4 @@
+        NewRHS->getOpcode() == ISD::TRUNCATE) {
+      NewLHS = SExtPromotedInteger(NewLHS);
+      NewRHS = SExtPromotedInteger(NewRHS);
+    } else {
----------------
Tim Northover wrote:
> 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.
Accepted and code is changed accordingly.

================
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;
----------------
Tim Northover wrote:
> 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).
Accepted and code is changed accordingly.

http://reviews.llvm.org/D4967






More information about the llvm-commits mailing list