[PATCH] D68852: [Attributor] Pointer privatization attribute (argument promotion)

[Hideto Ueno via Phabricator via llvm-commits]

uenoku added a comment.

Looks generally fine.
I couldn't imagine what is Pointer privatization at first hand. Could you add an example result of Pointer privatization? Like,

  int f(int* ptr){
   ...
  }
  =>
  int f(int p){
   int* ptr = &p;
   ...
  }

And I guess the current implementation always does privatization if possible. I think the cost may increase in some cases. What do you think about?



================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:2092
+/// A pointer is privatizable if it can be replaced by a new, private one.
+/// Privatizing pointer reduces the use count, interaction between unrelated
+/// code parts.
----------------
Could you add comments for the condition of whether the pointer can be replaced a private one?


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:2104
+
+  /// Return the type we can chose for a private copy of the underlying
+  /// value. None means it is not clear yet, nullptr means there is none.
----------------
nit: choose


================
Comment at: llvm/test/Transforms/ArgumentPromotion/chained.ll:18
+; ATTRIBUTOR-NEXT:  entry:
+; ATTRIBUTOR-NEXT:    [[Y:%.*]] = load i32*, i32** @G2, align 8
+; ATTRIBUTOR-NEXT:    [[Z:%.*]] = load i32, i32* [[Y]]
----------------
Please add FIXME here for `AAValueSimplify`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68852