[LSR] Change default of lsr-exp-narrow to false.

[dmgreen]

Summary

Commit 295704 introduced some benchmark regressions, hopefully characterised by the Shootout-C++/matrix regressions in [1]. The review claimed that this would be committed with the default for the option set to false, but if fact had it enabled. This reverses that default for the time being, in line with the review.

[1] http://llvm.org/perf/db_default/v4/nts/daily_report/2017/2/21?day_start=16

[dmgreen]

Hello Quentin,

I believe Evgeny is OOO at the moment. Do you have any objections to reversing this default, in lines with the review? It is not greatly urgent, so let me know if you think waiting is a better option.

Cheers
Dave

[dmgreen]

Sorry, I missed the dev list email and didn't realise it was intentionally left this way. Please ignore.

I'll try to get some more info for the mailing list tomorrow.

[qcolombet]

If it hasn't been done yet, I am in favor of this patch. I believe Evgeny got the feedback he was looking for.

[evstupac]

I've reverted it a few days ago.

[dmgreen]

Yes, this has already been handled, Thanks.

lib/Transforms/Scalar/LoopStrengthReduce.cpp

 
 // The flag adds instruction count to solutions cost comparision.
 static cl::opt<bool> InsnsCost(
   "lsr-insns-cost", cl::Hidden, cl::init(false),
   cl::desc("Add instruction count to a LSR cost model"));
 
 // Flag to choose how to narrow complex lsr solution
 static cl::opt<bool> LSRExpNarrow(
-  "lsr-exp-narrow", cl::Hidden, cl::init(true),
+  "lsr-exp-narrow", cl::Hidden, cl::init(false),
   cl::desc("Narrow LSR complex solution using"
            " expectation of registers number"));
 
 #ifndef NDEBUG
 // Stress test IV chain generation.
 static cl::opt<bool> StressIVChain(
   "stress-ivchain", cl::Hidden, cl::init(false),
   cl::desc("Stress test LSR IV chains"));

test/Transforms/LoopStrengthReduce/2013-01-14-ReuseCast.ll

 ; RUN: opt -loop-reduce -S < %s | FileCheck %s
 ;
 ; LTO of clang, which mistakenly uses no TargetLoweringInfo, causes a
 ; miscompile. ReuseOrCreateCast replace ptrtoint operand with undef.
 ; Reproducing the miscompile requires no triple, hence no "TTI".
 ; rdar://13007381
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
 ; Verify that nothing uses the "dead" ptrtoint from "undef".
 ; CHECK-LABEL: @VerifyDiagnosticConsumerTest(
 ; CHECK: bb:
 ; "dead" ptrpoint not emitted (or dead code eliminated) with
 ; current LSR cost model.
 ; CHECK-NOT: = ptrtoint i8* undef to i64
 ; CHECK: .lr.ph
-; CHECK: [[TMP:%[^ ]+]] = add i64 %4, 1
+; CHECK: [[TMP:%[^ ]+]] = add i64 %tmp5, 1
 ; CHECK: sub i64 [[TMP]], %tmp6
 ; CHECK: ret void
 define void @VerifyDiagnosticConsumerTest() unnamed_addr nounwind uwtable align 2 {
 bb:
   %tmp3 = call i8* @getCharData() nounwind
   %tmp4 = call i8* @getCharData() nounwind
   %tmp5 = ptrtoint i8* %tmp4 to i64
   %tmp6 = ptrtoint i8* %tmp3 to i64

test/Transforms/LoopStrengthReduce/ARM/ivchain-ARM.ll

 ; @foldedidx is an unrolled variant of this loop:
 ;  for (unsigned long i = 0; i < len; i += s) {
 ;    c[i] = a[i] + b[i];
 ;  }
 ; where 's' can be folded into the addressing mode.
 ; Consequently, we should *not* form any chains.
 ;
 ; A9: foldedidx:
-; A9: ldrb{{(.w)?}} {{r[0-9]|lr}}, [{{r[0-9]|lr}}, #403]
+; A9: ldrb{{(.w)?}} {{r[0-9]|lr}}, [{{r[0-9]|lr}}, #3]
 define void @foldedidx(i8* nocapture %a, i8* nocapture %b, i8* nocapture %c) nounwind ssp {
 entry:
   br label %for.body
 
 for.body:                                         ; preds = %for.body, %entry
   %i.07 = phi i32 [ 0, %entry ], [ %inc.3, %for.body ]
   %arrayidx = getelementptr inbounds i8, i8* %a, i32 %i.07
   %0 = load i8, i8* %arrayidx, align 1