llvm / llvm/llvm-project

Instcombine doesn't respect pointer's non-address bits

Open
#201,892 3 comments 0 reactions 0 assignees View on GitHub
llvm:instcombine
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

My LLM code inspector found a lot of "bugs" in instcombine around non-integral pointer types. For example:

```llvm
icmp eq (or (ptrtoint P), (ptrtoint Q)), 0
```

becomes

```llvm
%cmp1 = icmp eq ptr addrspace(4) %p, null
%cmp2 = icmp eq ptr addrspace(4) %q, null
%cmp = and i1 %cmp1, %cmp2
```

But the langref says `icmp ptr` only looks at the address bits, ignoring the non-address bits. Therefore this is not sound.

> If the operands are [pointer](https://llvm.org/docs/LangRef.html#t-pointer) typed, the address bits of the pointers are compared as if they were integers. Non-address bits or external state are not compared. That is, icmp on pointers is equivalent to icmp on the ptrtoaddr of the pointers.

I see that the langref says that

> Note: non-integral pointer types are a work in progress, and they should be considered experimental at this time.

But also Codex is telling me that AMDGPU uses this feature:

> p7:160...:32, p8:128...:48, p9:192...:32, marked ni:7:8:9. AS8 is documented as the newer representation for 128-bit buffer descriptors and may be converted to/from i128; AS7/AS9 are more experimental/unsupported.

Do we care about this? My guess is we don't because otherwise someone would have fixed these bugs, but wanted to check.

cc @arsenm @nikic

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.