[PATCH] D76966: [GlobalOpt/GlobalStatus][Mem2Reg] Handle PtrToInt passed as function call operand

[Dominic Chen via Phabricator via llvm-commits]

ddcc marked 2 inline comments as done.
ddcc added a comment.

I'm not sure yet if I'll need to support ptrtoint; I haven't rebuilt my benchmarks with the parent changes to see if optimizations are being inhibited. But I am more inclined to avoid it if possible since I'll need to refactor and call the code to infer pointer capture.



================
Comment at: llvm/test/Transforms/GlobalOpt/localize-fnattr.ll:95
+    ret i32 %e
+}
----------------
jdoerfert wrote:
> I'm curious, what value is passed to `@foo5` here? An alloca? What if foo5 does something like:
> 
> ```
> void foo5(int a) {
>   assert(a == &G5);
> }
> ```
> 
> If that can't happen, what if we pass `@G5` twice?
I don't believe this can happen, because the path from processGlobal() -> processInternalGlobal() -> isPointerValueDeadOnEntryToFunction() checks that the global has internal linkage and does not have multiple accessing functions.

When the localized alloca of `@G5` is passed in twice, they should compare equal.


================
Comment at: llvm/test/Transforms/Mem2Reg/fnattr.ll:60
+    ret i32 %c
+}
----------------
jdoerfert wrote:
> FWIW, the test case to break the current code looks like this:
> 
> ```
> define i32* toptr(i32 %a) readnone nowunind {
>   %ptr = inttoptr i32 %a, i32*
>   ret %ptr
> }
> 
> ; Do read from casted pointer argument
> define i32 @c() norecurse {
>     %g3 = alloca i32
>     store i32 42, i32 *%g3
>     %p = ptrtoint i32* %g3 to i32
>     %clone = call i32* @toptr(i32 %p)
>     store i32 1337, i32* %clone
>     %c = load i32, i32* %g3
>     ret i32 %c
> }
> ```
Ok, yeah, the capture check would be needed for this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76966