llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
25.96k stars 10.6k forks source link

Align attribute doesn't imply dereferenceability #90446

Open nunoplopes opened 2 weeks ago

nunoplopes commented 2 weeks ago

InstCombine does the following propagation of align(32) in the function argument to the load. However these align have different semantics. Align in load/store imply dereferenceability of the size rounded up to the alignment (per LangRef). Align in a function argument does not.

define i32 @foo2(ptr align(32) %a) {
  %#0 = load i32, ptr %a, align 4
  ret i32 %#0
}
=>
define i32 @foo2(ptr align(32) %a) {
  %#0 = load i32, ptr align(32) %a, align 32
  ret i32 %#0
}
Transformation doesn't verify! (unsound)
ERROR: Source is more defined than target

Example:
ptr align(32) %a = pointer(non-local, block_id=1, offset=0)
nikic commented 2 weeks ago

Relevant LangRef wording:

An alignment value higher than the size of the loaded type implies memory up to the alignment value bytes can be safely loaded without trapping in the default address space.

Agree this is a miscompile with the current spec, but it should possibly be fixed by changing the LangRef wording instead. I'm not sure it makes sense to have this requirement on the IR level (on the SDAG level it's a different matter -- we usually get a target-specific guarantee there because dereferenceability almost always works on page granularity.)

nunoplopes commented 2 weeks ago

This came after fixing https://github.com/AliveToolkit/alive2/issues/1028

Probably this kind of transformations are limited to the codegen prepare phase?