llvm / llvm-project

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

llc -combiner-alias-analysis reorders volatiles #10223

Closed llvmbot closed 1 year ago

llvmbot commented 13 years ago
Bugzilla Link 9851
Version 2.9
OS Linux
Attachments Test case for reordering of volatiles.
Reporter LLVM Bugzilla Contributor
CC @efriedma-quic,@sunfishcode

Extended Description

llc reorders accesses to volatiles if -combiner-alias-analysis is enabled, although volatiles should not be reordered with regard to each other.

Testcase, which miscompiles at least for sparc and ppc32: define i32 @​test() { %X = volatile load i32 inttoptr(i32 8 to i32) %Y = volatile load i32 inttoptr(i32 12 to i32) %Z = add i32 %Y, 1000 ret i32 %Z }

Correct code, from llc -march=ppc32 lwz 3, 8(0) lwz 3, 12(0) addi 3, 3, 1000 blr

Miscompiled code, from llc -march=ppc32 -combiner-alias-analysis lwz 3, 12(0) lwz 4, 8(0) addi 3, 3, 1000 blr

lattner commented 13 years ago

Eli is right, reordering these loads is invalid.

efriedma-quic commented 13 years ago

As far as I know reordering volatiles is perfectly valid. You need a memory barrier if you want to enforce ordering.

From LangRef: "The optimizers must not change the number of volatile operations or change their order of execution relative to other volatile operations."

llvmbot commented 13 years ago

As far as I know reordering volatiles is perfectly valid. You need a memory barrier if you want to enforce ordering.

efriedma-quic commented 13 years ago

Note that -combiner-alias-analysis is an experimental option, and issues are expected.

arsenm commented 1 year ago

Flag seems to have been renamed to -combiner-global-alias-analysis. I get the same result with and without it

define i32 @test() {
  %X = load volatile i32, i32* inttoptr(i32 8 to i32*)
  %Y = load volatile i32, i32* inttoptr(i32 12 to i32*)
  %Z = add i32 %Y, 1000
  ret i32 %Z
}