[PATCH] D10767: Fix shift legalization and lowering for big constants.

[Simon Pilgrim]

RKSimon added a subscriber: RKSimon.
RKSimon added a comment.

Hi Paweł - comments below.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:2180
@@ -2176,1 +2179,3 @@
+    }
     return ExpandShiftByConstant(N, CN->getZExtValue(), Lo, Hi);
+  }
----------------
I'm not sure about this - ExpandShiftByConstant already handles shift amounts out of range, setting the result to zero/signbit for logical/arithmetic shifts instead of undef.

I think it better to change ExpandShiftByConstant shift amount to take an APInt type instead of an unsigned and update its handling to compare using APInt.

If you want to make a case for returning undef for constant shift results it should be in a separate patch.

================
Comment at: test/CodeGen/X86/legalize-shl-vec.ll:3
@@ +2,3 @@
+; RUN: llc < %s -march=x86    | FileCheck %s
+
+; CHECK-LABEL: test1
----------------
It would be better if these tests checked the resultant codegen, rather than just test for a crash.


Repository:
  rL LLVM

http://reviews.llvm.org/D10767