Page MenuHomePhabricator

[GlobalOpt/GlobalStatus][Mem2Reg] Handle PtrToInt passed as function call operand
Needs ReviewPublic

Authored by ddcc on Mar 27 2020, 6:49 PM.



Support optimizing calls to readnone function with ptrtoint operand in global variable optimization and mem2reg

Depends on: D76894, D76965

Diff Detail

Event Timeline

ddcc created this revision.Mar 27 2020, 6:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 27 2020, 6:49 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
ddcc added a comment.Mar 27 2020, 6:54 PM

I'm not quite sure what the intended semantics for inaccessiblememonly and readnone are here. If a pointer operand is converted to an integer and passed as an argument to a function call, when can that pointer be optimized out? Does that function need to have either attribute, or does it not matter that an integer argument is casted from a pointer? This patch currently assumes that the attribute must still be present on the function (since it can't be set on a non-pointer argument).

jdoerfert added inline comments.Mar 27 2020, 11:41 PM

This is not sound. ptr2int + call can capture the pointer just fine so any use here has to be assumed capturing. That said, I would strongly recommend against looking through ptr2int if you don't have a good reason and good reasoning for soundness.

I looked at your tests, it seems you really want to support ptr2int. I see two options:

  1. Allow nocapture for non-pointer arguments, or
  2. Perform the sufficient nocapture check here if it is in integer. Since I don't want to duplicate the logic we should instead put it in a helper. Take a look at determineFunctionCaptureCapabilities in Attributor.cpp. The first check is to rule out capturing. We can make it a static member of AANoCapture in Attributor.h so we can call it here. Other places are fine too.

This is unrelated and not clear to me.


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?


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
ddcc marked 2 inline comments as done.Mar 28 2020, 3:43 PM

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.


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.


Ok, yeah, the capture check would be needed for this case.