[PATCH] D11886: [InstCombine] Move SSE2/AVX2 arithmetic vector shift folding to instcombiner

[Simon Pilgrim via llvm-commits]

RKSimon added a comment.

Thanks guys - I'll get an updated patch up as soon as I can.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:202
@@ -201,3 +201,3 @@
                                   InstCombiner::BuilderTy &Builder,
-                                  bool ShiftLeft) {
+                                  bool LogicalShift, bool ShiftLeft) {
   // Simplify if count is constant.
----------------
majnemer wrote:
> Would it make sense to assert `(LogicalShift || !ShiftLeft)` seeing as how there is no arithmetic left shift.  Alternatively, you could make that state impossible by construction by using an enum for the three states.
No problem - I'll add the assert.

================
Comment at: test/CodeGen/X86/combine-avx2-intrinsics.ll:6
@@ -5,3 +5,1 @@
 
-define <8 x i32> @test_psra_1(<8 x i32> %A) {
-  %1 = tail call <8 x i32> @llvm.x86.avx2.psrai.d(<8 x i32> %A, i32 3)
----------------
mkuper wrote:
> RKSimon wrote:
> > mkuper wrote:
> > > We still want to test these combines, right? (Only as part of InstCombine, not ISel)
> > I can add these shift accumulation tests as well if you wish but I will keep the simple tests in there too. The x86-vector-shifts.ll test file already has some general constant folding tests at the end that do various forms of accumulation.
> I'm just against removing (working) regression tests on principle. :-)
> But yes, I meant in addition, not instead of the simple test.
OK I'll transfer the tests over - note that I'll have to refactor them as they won't lower anymore.


Repository:
  rL LLVM

http://reviews.llvm.org/D11886