llvm / llvm/llvm-project

[mlir][LLVM] Pointer data layout compatibility ignores default address-space fallback

Open Beginner friendly
#222,879 1 comment 0 reactions 0 assignees View on GitHub
mlir
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

### Description

`LLVMPointerType::areCompatible()` does not correctly use the default
address-space pointer layout when an enclosing layout has no entry for a
specific address space introduced by a nested layout.

The function first searches `oldLayout` for an entry matching the new pointer's
address space. If none is found, it performs a second `llvm::find_if()` looking
for address space 0:

```cpp

if (it == oldLayout.end()) {
llvm::find_if(oldLayout, [&](DataLayoutEntryInterface entry) {
if (auto type = llvm::dyn_cast_if_present(entry.getKey())) {
return llvm::cast(type).getAddressSpace() == 0;
}
return false;
});
}
```

However, the result of this fallback search is discarded. it therefore
remains `oldLayout.end()`.

The compatibility check then continues using the hardcoded default pointer size
and alignment instead of the enclosing address-space-0 pointer layout.

Reproducer
```mlir
module attributes {
dlti.dl_spec = #dlti.dl_spec<
!llvm.ptr = dense<[32, 32, 32]> : vector<3xi64>
>
} {
module attributes {
dlti.dl_spec = #dlti.dl_spec<
!llvm.ptr<5> = dense<[32, 32, 32]> : vector<3xi64>
>
} {
}
}
```

Run with:

```bash
mlir-opt repro.mlir -o /dev/null
```

Actual behavior

The nested layout is rejected:

`error: data layout does not combine with layouts of enclosing ops`

Expected behavior

The nested layout should be compatible.

The enclosing layout has no explicit entry for address space 5, so the
compatibility check should fall back to its address-space-0 pointer entry.

Both the enclosing default pointer and the nested address-space-5 pointer have
a 32-bit size and compatible alignment.

Root cause

The fallback llvm::find_if() result is not assigned back to it.

Conceptually, the fallback should retain the iterator:

```cpp
if (it == oldLayout.end())
it = llvm::find_if(oldLayout, ...);
```

so that the compatibility check uses the enclosing default pointer layout.

Additional context

This was found while investigating #222356.

Two other independent problems found during that investigation are being
handled separately:

#222855 fixes the LLVM pointer data-layout value extraction crash.
#222874 fixes combination of supported builtin DLTI type entries.

This issue is independent of both fixes above.

Contributor guide

Open the contributing guide

Research direction

Start at LLVMPointerType::areCompatible() and inspect the fallback llvm::find_if() for address space 0. Run the provided reproducer with mlir-opt to confirm the nested layout is rejected, then verify it is accepted after the fallback iterator is retained and the compatibility check uses the enclosing default pointer layout.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.