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

[RISCV] Miscompilation of inline assembly #100779

Closed
shkoo opened this issue Jul 26, 2024 · 3 comments · Fixed by #100790
Closed

[RISCV] Miscompilation of inline assembly #100779

shkoo opened this issue Jul 26, 2024 · 3 comments · Fixed by #100790

Comments

@shkoo
Copy link
Contributor

shkoo commented Jul 26, 2024

It looks like LLC is miscompiling passing a register load to inline assembly:

target datalayout = "e-m:e-p:32:32-i64:64-n32-S128"
target triple = "riscv32"

@_ZN5repro9MY_BUFFER17hb0f674501d5980a6E = external global <{ [16 x i8] }>

define void @using_inout() {
start:
  %0 = tail call ptr asm sideeffect alignstack "ecall", "=&{x10},0,~{vtype},~{vl},~{vxsat},~{vxrm},~{memory}"(ptr @_ZN5repro9MY_BUFFER17hb0f674501d5980a6E)
  ret void
}

Running llc on this crashes with assertions enabled (https://godbolt.org/z/1zWaoxKKE).

Without assertions, it generates incorrect code (https://godbolt.org/z/43c4W44nY); notice that it silently discards the load of the low bits of _ZN5repro9MY_BUFFER17hb0f674501d5980a6E causing an incorrect value to be loaded in register a0:

using_inout:                            # @using_inout
        lui     a0, %hi(_ZN5repro9MY_BUFFER17hb0f674501d5980a6E)
        ecall
        ret

This regression looks like it happened somewhere between llvm 17 (https://godbolt.org/z/o6eWGx68o) and llvm 18 (https://godbolt.org/z/fMMd9Gf3W).

For more background, see rust-lang/rust#128212

@llvmbot
Copy link
Collaborator

llvmbot commented Jul 26, 2024

@llvm/issue-subscribers-backend-risc-v

Author: None (shkoo)

It looks like LLC is miscompiling passing a register load to inline assembly:
target datalayout = "e-m:e-p:32:32-i64:64-n32-S128"
target triple = "riscv32"

@<!-- -->_ZN5repro9MY_BUFFER17hb0f674501d5980a6E = external global &lt;{ [16 x i8] }&gt;

define void @<!-- -->using_inout() {
start:
  %0 = tail call ptr asm sideeffect alignstack "ecall", "=&amp;{x10},0,~{vtype},~{vl},~{vxsat},~{vxrm},~{memory}"(ptr @<!-- -->_ZN5repro9MY_BUFFER17hb0f674501d5980a6E)
  ret void
}

Running llc on this crashes with assertions enabled (https://godbolt.org/z/1zWaoxKKE).

Without assertions, it generates incorrect code (https://godbolt.org/z/43c4W44nY); notice that it silently discards the load of the low bits of _ZN5repro9MY_BUFFER17hb0f674501d5980a6E causing an incorrect value to be loaded in register a0:

using_inout:                            # @<!-- -->using_inout
        lui     a0, %hi(_ZN5repro9MY_BUFFER17hb0f674501d5980a6E)
        ecall
        ret

This regression looks like it happened somewhere between llvm 17 (https://godbolt.org/z/o6eWGx68o) and llvm 18 (https://godbolt.org/z/fMMd9Gf3W).

For more background, see rust-lang/rust#128212

@topperc topperc self-assigned this Jul 26, 2024
topperc added a commit to topperc/llvm-project that referenced this issue Jul 26, 2024
…s register in a non-memory constraint.

If the register is used by a non-memory constraint we should disable
the fold. Otherwise, we may leave CommonOffset unassigned.

Fixes llvm#100779.
@topperc topperc added this to the LLVM 19.X Release milestone Jul 27, 2024
@topperc
Copy link
Collaborator

topperc commented Jul 27, 2024

/cherry-pick c901b73

llvmbot pushed a commit to llvmbot/llvm-project that referenced this issue Jul 27, 2024
…s register in a non-memory constraint. (llvm#100790)

If the register is used by a non-memory constraint we should disable the
fold. Otherwise, we may leave CommonOffset unassigned.

Fixes llvm#100779.

(cherry picked from commit c901b73)
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 27, 2024

/pull-request #100843

tru pushed a commit to llvmbot/llvm-project that referenced this issue Jul 27, 2024
…s register in a non-memory constraint. (llvm#100790)

If the register is used by a non-memory constraint we should disable the
fold. Otherwise, we may leave CommonOffset unassigned.

Fixes llvm#100779.

(cherry picked from commit c901b73)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment