[PATCH] D79299: [TRE][NFC] Refactor in preparation for removal of isDynamicConstant

[Layton Kifer via Phabricator via llvm-commits]

laytonio created this revision.
laytonio added reviewers: nlewycky, lattner, Carrot, efriedma.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

As seen in the below example, the requirement that accumulators are initialized to a "dynamic" constant, is unnecessary. We can instead initialize the accumulator with the identity constant for that accumulation operation and perform the final accumulation before any returns in the function. In addition, the current implementation of isDynamicConstant has a bug that sometimes causes us to preform an invalid transform when trying to propagate a "dynamic" constant used in a switch statement. This bug can also be seen in the below example.

This refactor is in preparation for the removal of this limitation and bug. This would otherwise require passing additional shared state in and out of most these functions which already have bloated parameter lists.

Example provided by efriedam:

Input

  define i32 @f() local_unnamed_addr {
  entry:
    %call = call i32 @g()
    switch i32 %call, label %sw.default [
      i32 1, label %cleanup
      i32 2, label %cleanup
    ]
  
  sw.default:
    %call1 = call i32 @f()
    %add = add nsw i32 %call1, 1
    ret i32 %add
  
  cleanup:
    ret i32 %call
  }
  
  declare i32 @g()

Current, invalid -tailcallelim output on trunk:

  define i32 @f() local_unnamed_addr {
  entry:
    br label %tailrecurse
  
  tailrecurse:                                      ; preds = %sw.default, %entry
    %accumulator.tr = phi i32 [ %call, %entry ], [ %add, %sw.default ]
    %call = tail call i32 @g()
    switch i32 %call, label %sw.default [
      i32 1, label %cleanup
      i32 2, label %cleanup
    ]
  
  sw.default:                                       ; preds = %tailrecurse
    %add = add nsw i32 %accumulator.tr, 1
    br label %tailrecurse
  
  cleanup:                                          ; preds = %tailrecurse, %tailrecurse
    ret i32 %accumulator.tr
  }
  
  declare i32 @g()

Alternative transform:

  define i32 @f() local_unnamed_addr {
  entry:
    br label %tailrecurse
  
  tailrecurse:                                      ; preds = %sw.default, %entry
    %accumulator.tr = phi i32 [ 0, %entry ], [ %add, %sw.default ]
    %call = tail call i32 @g()
    switch i32 %call, label %sw.default [
      i32 1, label %cleanup
      i32 2, label %cleanup
    ]
  
  sw.default:                                       ; preds = %tailrecurse
    %add = add nsw i32 %accumulator.tr, 1
    br label %tailrecurse
  
  cleanup:                                          ; preds = %tailrecurse, %tailrecurse
    %accumulator.ret = add i32 %accumulator.tr, %call
    ret i32 %accumulator.ret
  }
  
  declare i32 @g()


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79299

Files:
  llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79299.261675.patch
Type: text/x-patch
Size: 32802 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200503/8c45d3b9/attachment.bin>