[PATCH] D13161: [PATCH, PR24373] Combine shifts for x86

[Simon Pilgrim via llvm-commits]

RKSimon requested changes to this revision.
RKSimon added a comment.
This revision now requires changes to proceed.

OK - it looks like some additional changes are required - comments below. I agree that the scheduling issue isn't directly tied to this patch, but you need to at least create a bugzilla with a minimal repro.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:24204
@@ +24203,3 @@
+      N0.getOpcode() != ISD::SHL ||
+      N0.getOperand(1).getOpcode() != ISD::Constant)
+    return SDValue();
----------------
I think you will need to add a test for N0.hasOneUse() as well here - otherwise there is a likely chance that the SHL will still need to be performed.

================
Comment at: test/CodeGen/X86/sar_fold.ll:3
@@ +2,3 @@
+
+define i32 @shl16sar15(i32 %a) #0 {
+; CHECK-LABEL: shl16sar15:
----------------
Please can you replace the march with a mtriple?

================
Comment at: test/CodeGen/X86/sar_fold64.ll:2
@@ +1,3 @@
+; RUN: llc < %s -O2 -march=x86-64 | FileCheck %s
+
+define i32 @shl48sar47(i64 %a) #0 {
----------------
Please can you replace the march with a mtriple?

================
Comment at: test/CodeGen/X86/vector-sext.ll:1615
@@ -1621,3 +1614,3 @@
 
 define <16 x i16> @load_sext_16i1_to_16i16(<16 x i1> *%ptr) {
 ; SSE2-LABEL: load_sext_16i1_to_16i16:
----------------
Annotating with nounwind readnone should help here.


Repository:
  rL LLVM

http://reviews.llvm.org/D13161