[PATCH] D78582: [InstCombine] substitute equivalent constant to reduce logic-of-icmps

Fri Apr 24 10:48:49 PDT 2020

lebedev.ri added a comment.

In D78582#1999956 <https://reviews.llvm.org/D78582#1999956>, @nikic wrote:

> In D78582#1999708 <https://reviews.llvm.org/D78582#1999708>, @spatel wrote:
>
> > In D78582#1999502 <https://reviews.llvm.org/D78582#1999502>, @nikic wrote:
> >
> > > It looks like this caused a +0.40% text size increase <http://llvm-compile-time-tracker.com/compare.php?from=7003a1da37b2aae5b17460922efde9efde2c229d&to=c79227cabb3059b4f01c07b6f8bfc7986a71d156&stat=size-text> on kimwitu++. There's a corresponding > 10% compile-time increase <http://llvm-compile-time-tracker.com/compare.php?from=7003a1da37b2aae5b17460922efde9efde2c229d&to=c79227cabb3059b4f01c07b6f8bfc7986a71d156&stat=instructions&details=on> on the main.cc file, so that's likely the relevant one.
> > >
> > > If you have time, it would be good to double check what's going on there, as text size increase usually means we're losing optimizations.
> >
> >
> > I'm going to expose my lack of git knowledge here, but how do we know this commit is to blame for the difference?
>
>
> Better link: http://llvm-compile-time-tracker.com/compare.php?from=7003a1da37b2aae5b17460922efde9efde2c229d&to=62da6ecea298739ad59c0563ce6d9493804ef1f0&stat=size-text This one only contains this change in the diff range. (The previous one had an additional MLIR change, which can't make a difference.)
>
> > How do I determine the build flags that are needed to repro?
>
> The flags are determined by test-suite `cmake/caches/O3.cmake` configuration. I get the flags by running `ninja kc -v` and sticking `-emit-llvm` on the right one, which would be for me:
>
> > /home/nikic/llvm-project/build/bin/clang++  -DNDEBUG  -O3   -w -Werror=date-time -I/home/nikic/llvm-test-suite/MultiSource/Applications/kimwitu++ -DYYDEBUG=1 -MD -MT MultiSource/Applications/kimwitu++/CMakeFiles/kc.dir/util.cc.o -MF MultiSource/Applications/kimwitu++/CMakeFiles/kc.dir/main.cc.o.d -o MultiSource/Applications/kimwitu++/CMakeFiles/kc.dir/main.cc.o -c ../MultiSource/Applications/kimwitu++/main.cc -emit-llvm
>
>
>
> > I tried reverting this patch locally and compiling /testsuite/trunk/MultiSource/Applications/kimwitu++/main.cc, and the IR is identical to pre-revert compiled at -O2 or -O3.
>
> Hm, are you sure you reverted the right one? I also didn't see a difference, until I realized I reverted the InstSimplify change instead of the InstCombine change, duh. After fixing that, I get these two files and diff: https://gist.github.com/nikic/ecbe2482367be25b17ab982c2346b661

I'm looking into this, and i believe those are improvements, not regressions.
F11790613: old.ll <https://reviews.llvm.org/F11790613> F11790616: new.ll <https://reviews.llvm.org/F11790616>
As it can be seen on llvm-diff, we inlined more:

  $ llvm-diff-11 old.ll new.ll 
  function @_ZStplIcSt11char_traitsIcESaIcEENSt7__cxx1112basic_stringIT_T0_T1_EERKS8_PKS5_ exists only in left module
  function @_ZStplIcSt11char_traitsIcESaIcEENSt7__cxx1112basic_stringIT_T0_T1_EERKS8_SA_ exists only in left module
  function @_ZSt19__throw_logic_errorPKc exists only in left module
  in function main:
    in block %entry:
      >   %__dnew.i.i.i.i.i2584 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i2481 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i2448 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i2360 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i2319 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i2216 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i2099 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i2046 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i1873 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i1840 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i1789 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i1736 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i1561 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i1528 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i1477 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i1424 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i1237 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i1204 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i1171 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i1118 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i915 = alloca i64, align 8
      >   %__dnew.i.i.i.i.i = alloca i64, align 8
      >   %__dnew.i.i.i.i.i692.i = alloca i64, align 8
      >   %__dnew.i.i.i.i.i662.i = alloca i64, align 8
      >   %__dnew.i.i.i.i.i633.i = alloca i64, align 8
      >   %__dnew.i.i.i.i.i.i = alloca i64, align 8
    in block %for.body196.lr.ph.i:
      >   %106 = bitcast %"class.std::__cxx11::basic_string.4"* %ref.tmp222.i to i8*
      >   %107 = getelementptr inbounds %"class.std::__cxx11::basic_string.4", %"class.std::__cxx11::basic_string.4"* %ref.tmp222.i, i64 0, i32 2
          %106 = bitcast %"class.std::__cxx11::basic_string"* %ref.tmp222.i to i8*
      >   %109 = bitcast i64* %__dnew.i.i.i.i.i.i to i8*
      >   %110 = bitcast %union.anon.6* %107 to i8*
          %_M_p.i.i625.i = getelementptr inbounds %"class.std::__cxx11::basic_string", %"class.std::__cxx11::basic_string"* %ref.tmp222.i, i64 0, i32 0, i32 0
      >   %_M_allocated_capacity.i.i.i.i.i.i625.i = getelementptr inbounds %"class.std::__cxx11::basic_string.4", %"class.std::__cxx11::basic_string.4"* %ref.tmp222.i, i64 0, i32 2, i32 0
      >   %_M_string_length.i.i.i.i.i.i.i.i = getelementptr inbounds %"class.std::__cxx11::basic_string.4", %"class.std::__cxx11::basic_string.4"* %ref.tmp222.i, i64 0, i32 1
      >   %111 = bitcast %"class.std::__cxx11::basic_string.4"* %ref.tmp227.i to i8*
      >   %112 = getelementptr inbounds %"class.std::__cxx11::basic_string.4", %"class.std::__cxx11::basic_string.4"* %ref.tmp227.i, i64 0, i32 2
          %107 = bitcast %"class.std::__cxx11::basic_string"* %ref.tmp227.i to i8*
      >   %114 = bitcast i64* %__dnew.i.i.i.i.i633.i to i8*
      >   %115 = bitcast %union.anon.6* %112 to i8*
          %_M_p.i.i626.i = getelementptr inbounds %"class.std::__cxx11::basic_string", %"class.std::__cxx11::basic_string"* %ref.tmp227.i, i64 0, i32 0, i32 0
      >   %_M_allocated_capacity.i.i.i.i.i.i638.i = getelementptr inbounds %"class.std::__cxx11::basic_string.4", %"class.std::__cxx11::basic_string.4"* %ref.tmp227.i, i64 0, i32 2, i32 0
      >   %_M_string_length.i.i.i.i.i.i.i644.i = getelementptr inbounds %"class.std::__cxx11::basic_string.4", %"class.std::__cxx11::basic_string.4"* %ref.tmp227.i, i64 0, i32 1
      >   %116 = bitcast %"class.std::__cxx11::basic_string.4"* %ref.tmp235.i to i8*
      >   %117 = getelementptr inbounds %"class.std::__cxx11::basic_string.4", %"class.std::__cxx11::basic_string.4"* %ref.tmp235.i, i64 0, i32 2
          %108 = bitcast %"class.std::__cxx11::basic_string"* %ref.tmp235.i to i8*
      >   %119 = bitcast i64* %__dnew.i.i.i.i.i662.i to i8*
      >   %120 = bitcast %union.anon.6* %117 to i8*
          %_M_p.i.i627.i = getelementptr inbounds %"class.std::__cxx11::basic_string", %"class.std::__cxx11::basic_string"* %ref.tmp235.i, i64 0, i32 0, i32 0
      >   %_M_allocated_capacity.i.i.i.i.i.i667.i = getelementptr inbounds %"class.std::__cxx11::basic_string.4", %"class.std::__cxx11::basic_string.4"* %ref.tmp235.i, i64 0, i32 2, i32 0
      >   %_M_string_length.i.i.i.i.i.i.i673.i = getelementptr inbounds %"class.std::__cxx11::basic_string.4", %"class.std::__cxx11::basic_string.4"* %ref.tmp235.i, i64 0, i32 1
          %109 = bitcast %"class.std::__cxx11::basic_string"* %ref.tmp244.i to i8*
      <   %_M_p.i.i628.i = getelementptr inbounds %"class.std::__cxx11::basic_string", %"class.std::__cxx11::basic_string"* %ref.tmp244.i, i64 0, i32 0, i32 0
          %110 = getelementptr inbounds %"class.std::__cxx11::basic_string", %"class.std::__cxx11::basic_string"* %ref.tmp244.i, i64 0, i32 2
      >   %123 = bitcast %"class.std::__cxx11::basic_string.4"* %ref.tmp244.i to %union.anon.6**
      >   %124 = bitcast i64* %__dnew.i.i.i.i.i692.i to i8*
          %arraydecay.i.i.i.i630.i = bitcast %union.anon* %110 to i8*
      >   %_M_p.i18.i.i.i.i.i696.i = getelementptr inbounds %"class.std::__cxx11::basic_string.4", %"class.std::__cxx11::basic_string.4"* %ref.tmp244.i, i64 0, i32 0, i32 0
      >   %_M_allocated_capacity.i.i.i.i.i.i697.i = getelementptr inbounds %"class.std::__cxx11::basic_string.4", %"class.std::__cxx11::basic_string.4"* %ref.tmp244.i, i64 0, i32 2, i32 0
      >   %_M_string_length.i.i.i.i.i.i.i703.i = getelementptr inbounds %"class.std::__cxx11::basic_string.4", %"class.std::__cxx11::basic_string.4"* %ref.tmp244.i, i64 0, i32 1
      <   %111 = getelementptr inbounds %"class.std::__cxx11::basic_string", %"class.std::__cxx11::basic_string"* %ref.tmp235.i, i64 0, i32 2
      <   %arraydecay.i.i.i.i636.i = bitcast %union.anon* %111 to i8*
      <   %112 = getelementptr inbounds %"class.std::__cxx11::basic_string", %"class.std::__cxx11::basic_string"* %ref.tmp227.i, i64 0, i32 2
      <   %arraydecay.i.i.i.i642.i = bitcast %union.anon* %112 to i8*
      <   %113 = getelementptr inbounds %"class.std::__cxx11::basic_string", %"class.std::__cxx11::basic_string"* %ref.tmp222.i, i64 0, i32 2
      <   %arraydecay.i.i.i.i648.i = bitcast %union.anon* %113 to i8*
    in block %if.else221.i:
      >   call void @llvm.lifetime.start.p0i8(i64 32, i8* nonnull %106) #22
      >   store %union.anon.6* %107, %union.anon.6** %108, align 8, !tbaa !21, !alias.scope !41
      >   %157 = load i8*, i8** getelementptr inbounds (%struct.cmdline_options.3, %struct.cmdline_options.3* @g_options, i64 0, i32 22, i32 0, i32 0), align 8, !tbaa !23, !noalias !41
      >   %158 = load i64, i64* getelementptr inbounds (%struct.cmdline_options.3, %struct.cmdline_options.3* @g_options, i64 0, i32 22, i32 1), align 8, !tbaa !10, !noalias !41
      >   call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %109) #22, !noalias !41
      >   store i64 %158, i64* %__dnew.i.i.i.i.i.i, align 8, !tbaa !44, !noalias !41
      >   %cmp3.i.i.i.i.i.i = icmp ugt i64 %158, 15
      >   br i1 %cmp3.i.i.i.i.i.i, label %if.then4.i.i.i.i.i.i, label %if.end6.i.i.i.i.i.i
      <   call void @llvm.lifetime.start.p0i8(i64 32, i8* nonnull %106) #23
      <   call void @_ZStplIcSt11char_traitsIcESaIcEENSt7__cxx1112basic_stringIT_T0_T1_EERKS8_PKS5_(%"class.std::__cxx11::basic_string"* nonnull sret align 8 %ref.tmp222.i, %"class.std::__cxx11::basic_string"* dereferenceable(32) getelementptr inbounds (%struct.cmdline_options, %struct.cmdline_options* @g_options, i64 0, i32 22), i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str.74, i64 0, i64 0))
      <   %145 = load i8*, i8** %_M_p.i.i625.i, align 8, !tbaa !23
      <   %call224.i = call i32 @strcmp(i8* nonnull dereferenceable(1) %mybasename.0.i624.i, i8* nonnull dereferenceable(1) %145) #22
      <   %cmp225.i = icmp eq i32 %call224.i, 0
      <   br i1 %cmp225.i, label %cleanup.done284.i, label %lor.lhs.false226.i
    in block %if.end193:
      >   %598 = getelementptr inbounds %"class.std::__cxx11::basic_string.4", %"class.std::__cxx11::basic_string.4"* %ref.tmp, i64 0, i32 2
      >   %599 = bitcast %"class.std::__cxx11::basic_string.4"* %ref.tmp to %union.anon.6**
      >   store %union.anon.6* %598, %union.anon.6** %599, align 8, !tbaa !21, !alias.scope !109
      >   %600 = load i8*, i8** getelementptr inbounds (%struct.cmdline_options.3, %struct.cmdline_options.3* @g_options, i64 0, i32 22, i32 0, i32 0), align 8, !tbaa !23, !noalias !109
      >   %601 = load i64, i64* getelementptr inbounds (%struct.cmdline_options.3, %struct.cmdline_options.3* @g_options, i64 0, i32 22, i32 1), align 8, !tbaa !10, !noalias !109
      >   %602 = bitcast i64* %__dnew.i.i.i.i.i to i8*
      >   call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %602) #22, !noalias !109
      >   store i64 %601, i64* %__dnew.i.i.i.i.i, align 8, !tbaa !44, !noalias !109
      >   %cmp3.i.i.i.i.i = icmp ugt i64 %601, 15
      >   br i1 %cmp3.i.i.i.i.i, label %if.then4.i.i.i.i.i, label %if.end.if.end6_crit_edge.i.i.i.i.i
      <   call void @_ZStplIcSt11char_traitsIcESaIcEENSt7__cxx1112basic_stringIT_T0_T1_EERKS8_PKS5_(%"class.std::__cxx11::basic_string"* nonnull sret align 8 %ref.tmp, %"class.std::__cxx11::basic_string"* dereferenceable(32) getelementptr inbounds (%struct.cmdline_options, %struct.cmdline_options* @g_options, i64 0, i32 22), i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str.12, i64 0, i64 0))
      <   invoke void @_ZN14kc_filePrinter4initEPKcS1_RKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE(%class.kc_filePrinter* nonnull @v_hfile_printer, i8* getelementptr inbounds ([10 x i8], [10 x i8]* @.str.10, i64 0, i64 0), i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.str.11, i64 0, i64 0), %"class.std::__cxx11::basic_string"* nonnull dereferenceable(32) %ref.tmp)
            to label %invoke.cont unwind label %lpad

  in function _ZN2kcL8openfileEPKcS1_:
    in block %entry:
      >   %__dnew.i.i.i.i.i = alloca i64, align 8
    in block %if.else:
      >   %2 = getelementptr inbounds %"class.std::__cxx11::basic_string.4", %"class.std::__cxx11::basic_string.4"* %ref.tmp2, i64 0, i32 2
      >   %3 = bitcast %"class.std::__cxx11::basic_string.4"* %ref.tmp2 to %union.anon.6**
      >   store %union.anon.6* %2, %union.anon.6** %3, align 8, !tbaa !2, !alias.scope !7
      >   %4 = load i8*, i8** getelementptr inbounds (%struct.cmdline_options.3, %struct.cmdline_options.3* @g_options, i64 0, i32 24, i32 0, i32 0), align 8, !tbaa !10, !noalias !7
      >   %5 = load i64, i64* getelementptr inbounds (%struct.cmdline_options.3, %struct.cmdline_options.3* @g_options, i64 0, i32 24, i32 1), align 8, !tbaa !13, !noalias !7
      >   %6 = bitcast i64* %__dnew.i.i.i.i.i to i8*
      >   call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %6) #21, !noalias !7
      >   store i64 %5, i64* %__dnew.i.i.i.i.i, align 8, !tbaa !14, !noalias !7
      >   %cmp3.i.i.i.i.i = icmp ugt i64 %5, 15
      >   br i1 %cmp3.i.i.i.i.i, label %if.then4.i.i.i.i.i, label %if.end.if.end6_crit_edge.i.i.i.i.i
      <   call void @_ZStplIcSt11char_traitsIcESaIcEENSt7__cxx1112basic_stringIT_T0_T1_EERKS8_PKS5_(%"class.std::__cxx11::basic_string"* nonnull sret align 8 %ref.tmp2, %"class.std::__cxx11::basic_string"* dereferenceable(32) getelementptr inbounds (%struct.cmdline_options, %struct.cmdline_options* @g_options, i64 0, i32 24), i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.str.15, i64 0, i64 0))
      <   %call.i.i.i = call i64 @strlen(i8* nonnull dereferenceable(1) %file) #22, !noalias !2
      <   %_M_string_length.i.i.i.i = getelementptr inbounds %"class.std::__cxx11::basic_string", %"class.std::__cxx11::basic_string"* %ref.tmp2, i64 0, i32 1
      <   %2 = load i64, i64* %_M_string_length.i.i.i.i, align 8, !tbaa !5, !noalias !2
      <   %sub3.i.i.i = sub i64 4611686018427387903, %2
      <   %cmp.i.i.i = icmp ult i64 %sub3.i.i.i, %call.i.i.i
      <   br i1 %cmp.i.i.i, label %if.then.i.i.i, label %_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEE6appendEPKc.exit.i

  in function _GLOBAL__sub_I_main.cc:
    in block %entry:
      >   tail call void @_ZNSt8ios_base4InitC1Ev(%"class.std::ios_base::Init.0"* nonnull @_ZStL8__ioinit)
      >   %0 = tail call i32 @__cxa_atexit(void (i8*)* bitcast (void (%"class.std::ios_base::Init.0"*)* @_ZNSt8ios_base4InitD1Ev to void (i8*)*), i8* getelementptr inbounds (%"class.std::ios_base::Init.0", %"class.std::ios_base::Init.0"* @_ZStL8__ioinit, i64 0, i32 0), i8* nonnull @__dso_handle) #21
      <   tail call void @_ZNSt8ios_base4InitC1Ev(%"class.std::ios_base::Init"* nonnull @_ZStL8__ioinit)
      <   %0 = tail call i32 @__cxa_atexit(void (i8*)* bitcast (void (%"class.std::ios_base::Init"*)* @_ZNSt8ios_base4InitD1Ev to void (i8*)*), i8* getelementptr inbounds (%"class.std::ios_base::Init", %"class.std::ios_base::Init"* @_ZStL8__ioinit, i64 0, i32 0), i8* nonnull @__dso_handle) #22

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78582/new/

https://reviews.llvm.org/D78582