[PATCH] D123669: [ArgPromotion] Use SmallSetVector to traverse values

[Pavel Samolysov via Phabricator via llvm-commits]

psamolysov added a comment.

The cycle between an instruction and its users is possible for PHI nodes: only they may reference their own value but the loop in the patch handles no instructions but `GEP`, `bitcast` and `load`.

For example for the following input:

  define internal i32* @callee(i32* noundef %0, i32* noundef %1) #0 {
  2:
    br label %3
  3:
    %4 = phi i32* [ %4, %3 ], [ %1, %2 ]
    br label %3
  
    ret i32* %4
  }

The pass just writes into the debug stream: `ArgPromotion of i32* %1 failed: unknown user   %4 = phi i32* [ %4, %3 ], [ %1, %2 ]` and doesn't fell into a loop.

Also, there can be a situation when the graph looks like a diamond:

  %0 = ...
  %1 = ... %0 ...
  %2 = ... %0 ...
  %3 = ... %1 ... %2 ... ; might be handled twice
  %4 = load ... %3 ...

In this case if there was no `Visited` set, instruction `%3` would be handled twice, but because only the `GEP`, `bitcast` and `load` instructions are handled and no one of them can have more than a single pointer argument (I'm not sure about `GEP` but as I see even `GEP` has one pointer argument too), the promotion just fails one time on an unknown user:

  %3 = bitcast i32* %0 to float*
  %4 = bitcast i32* %1 to float*
  %5 = icmp ugt float* %3, %4
  %6 = select i1 %5, float* %3, float* %4
  %7 = load float, float* %6, align 4, !tbaa !4
  ...

output:

  ArgPromotion of i32* %0 failed: unknown user   %5 = icmp ugt float* %3, %4

So, the optimization (to remove the `Visited` set) looks like making sense here.  What is your opinion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123669