[PATCH] D149768: [ArgumentPromotion] Bail if any callers are minsize

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

we were seeing size increases with code like the following

  define internal i32 @f1(ptr %p) {
    %i = load i32, ptr %p, align 4
    ret i32 %i
  }
  
  define i32 @g1(ptr %p) {
    %i = call i32 @f1(ptr %p)
    %q = call i32 @f1(ptr %p)
    %w = call i32 @f1(ptr %p)
    %e = call i32 @f1(ptr %p)
    %r = call i32 @f1(ptr %p)
    %t = call i32 @f1(ptr %p)
    %y = call i32 @f1(ptr %p)
    ret i32 %i
  }

becoming

  define internal i32 @f1(i32 %p.0.val) {
    ret i32 %p.0.val
  }
  
  define i32 @g1(ptr %p) {
    %p.val6 = load i32, ptr %p, align 4
    %i = call i32 @f1(i32 %p.val6)
    %p.val5 = load i32, ptr %p, align 4
    %1 = call i32 @f1(i32 %p.val5)
    %p.val4 = load i32, ptr %p, align 4
    %2 = call i32 @f1(i32 %p.val4)
    %p.val3 = load i32, ptr %p, align 4
    %3 = call i32 @f1(i32 %p.val3)
    %p.val2 = load i32, ptr %p, align 4
    %4 = call i32 @f1(i32 %p.val2)
    %p.val1 = load i32, ptr %p, align 4
    %5 = call i32 @f1(i32 %p.val1)
    %p.val = load i32, ptr %p, align 4
    %6 = call i32 @f1(i32 %p.val)
    ret i32 %i
  }

I did add a TODO here for a potential place for improvement, but even with that it doesn't take into account the fact that we can likely simplify more with the value now in SSA form after mem2reg. but maybe that TODO is worth pursuing anyway

but another thing to consider is that we only recently started running argpromo for -O1/2/s/z. I can see two cases where this patch would regress -Oz code size. one is running an -O3 post-link with an -Oz pre-link, which wouldn't make sense in general. the second is that https://reviews.llvm.org/D148269 actually helped with -Oz code size in your case, then this patch regressed it back to where it was before

otherwise if I'm missing something and you really need to revert this, you can revert https://reviews.llvm.org/D148269 and this patch together


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149768