Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missed optimization: constant propagation with a pointer to a const local #59694

Open
haberman opened this issue Dec 24, 2022 · 3 comments
Open

Comments

@haberman
Copy link
Contributor

Repro:

void ExternFunc(const int*);

int Bad() {
  const int i = 0;
  const int* pi = &i;
  ExternFunc(pi);
  return *pi;
}

int Good() {
  const int i = 0;
  ExternFunc(&i);
  return i;
}

Output from trunk (Godbolt link: https://godbolt.org/z/KT8dPM9ac):

Bad:                                    # @Bad
        push    rax
        mov     dword ptr [rsp + 4], 0
        lea     rdi, [rsp + 4]
        call    ExternFunc@PLT
        mov     eax, dword ptr [rsp + 4]
        pop     rcx
        ret
Good:                                   # @Good
        push    rax
        mov     dword ptr [rsp + 4], 0
        lea     rdi, [rsp + 4]
        call    ExternFunc@PLT
        xor     eax, eax
        pop     rcx
        ret

In Good(), Clang correctly performs constant propagation on the local i, even though the pointer escapes, because i is a const object that cannot be modified by ExternFunc() without invoking UB.

In Bad(), we have basically the same scenario, except in this case Clang does not perform the constant propagation. pi is not merely a const pointer, it is a pointer to a const object, and therefore the value it points to is guaranteed to be stable even across a function call where the pointer escapes. But Clang doesn't seem to realize this, and re-loads i after the function call, even though i is a const object.

@haberman
Copy link
Contributor Author

The harm in this example is relatively minor (one extra load), but in my actual code I was relying on constant propagation to eliminate many other branches that test the value of *pi. So this impact of this missed optimization on my code is rather large.

@efriedma-quic
Copy link
Collaborator

As a workaround in your code, you could make the constant "static const".

If you look at the LLVM IR generated by clang, in the "good" case, the constant is actually evaluated by the frontend. In the "bad" case, it isn't, and clang doesn't currently try to represent the fact that the variable is const in LLVM IR. (llvm.invariant.start might be usable for this.)

@haberman
Copy link
Contributor Author

As a workaround in your code, you could make the constant "static const".

I want to avoid having the constant take up any space in the binary (there are a lot of these).

Another workaround I found was:

void ExternFunc(const int*);

int Bad() {
  const int i = 0;
  const int* pi = &i;
  ExternFunc(pi);
  if (*pi != i) __builtin_unreachable();
  return *pi;
}

It looks a bit silly in this case, but in the real code this is split across several inline functions, and it is more reasonable there.

llvm.invariant.start might be usable for this

That sounds great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants