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

AArch64 with global isel miscompile #78477

Closed
tanmaytirpankar opened this issue Jan 17, 2024 · 5 comments · Fixed by #82306
Closed

AArch64 with global isel miscompile #78477

tanmaytirpankar opened this issue Jan 17, 2024 · 5 comments · Fixed by #82306

Comments

@tanmaytirpankar
Copy link

The following LLVM IR:

define i32 @f(ptr %0) {
  %2 = load <2 x i32>, ptr %0, align 8
  store <4 x i32> zeroinitializer, ptr %0, align 16
  %3 = extractelement <2 x i32> %2, i64 0
  ret i32 %3
}

with SDAG lowers to

../../llvm-clone/llvm/build-release/bin/llc -march=aarch64 foo.ll -o foo1.ll
f:
	ldr	d0, [x0]
	mov	x8, x0
	str	xzr, [x0, #8]
	str	xzr, [x8]
	fmov	w0, s0
	ret

but with -global-isel it lowers to

../../llvm-clone/llvm/build-release/bin/llc -march=aarch64 -global-isel foo.ll -o foo1.ll
f:
	mov	x8, x0
	str	xzr, [x0]
	and	x0, xzr, #0xffffffff
	str	xzr, [x8, #8]
	ret

The SDAG version saves the value from memory pointed to by register x0 first to d0 and then to w0 and zeros the memory content. The global-isel version on the other hand does not save the value and simply zeros the memory content. The final state of w0 differs in both versions causing the return value to differ.

cc @regehr

@llvmbot
Copy link
Member

llvmbot commented Jan 17, 2024

@llvm/issue-subscribers-backend-aarch64

Author: Tanmay Tirpankar (tanmaytirpankar)

The following LLVM IR: ``` define i32 @f(ptr %0) { %2 = load <2 x i32>, ptr %0, align 8 store <4 x i32> zeroinitializer, ptr %0, align 16 %3 = extractelement <2 x i32> %2, i64 0 ret i32 %3 } ``` with `SDAG` lowers to ``` ../../llvm-clone/llvm/build-release/bin/llc -march=aarch64 foo.ll -o foo1.ll f: ldr d0, [x0] mov x8, x0 str xzr, [x0, #8] str xzr, [x8] fmov w0, s0 ret ``` but with `-global-isel` it lowers to ``` ../../llvm-clone/llvm/build-release/bin/llc -march=aarch64 -global-isel foo.ll -o foo1.ll f: mov x8, x0 str xzr, [x0] and x0, xzr, #0xffffffff str xzr, [x8, #8] ret ``` The `SDAG` version saves the value from memory pointed to by register `x0` first to `d0` and then to `w0` and zeros the memory content. The `global-isel` version on the other hand does not save the value and simply zeros the memory content. The final state of `w0` differs in both versions causing the return value to differ.

cc @regehr

@tschuett
Copy link

I tried llc -march=aarch64 -global-isel -stop-after=irtranslator foo.ll -o foo.mir and got:

 bb.1 (%ir-block.1):
    liveins: $x0

    %0:_(p0) = COPY $x0
    %3:_(s32) = G_CONSTANT i32 0
    %2:_(<4 x s32>) = G_BUILD_VECTOR %3(s32), %3(s32), %3(s32), %3(s32)
    %5:_(s64) = G_CONSTANT i64 0
    %1:_(<2 x s32>) = G_LOAD %0(p0) :: (load (<2 x s32>) from %ir.0)
    G_STORE %2(<4 x s32>), %0(p0) :: (store (<4 x s32>) into %ir.0)
    %4:_(s32) = G_EXTRACT_VECTOR_ELT %1(<2 x s32>), %5(s64)
    $w0 = COPY %4(s32)
    RET_ReallyLR implicit $w0

@tschuett
Copy link

I tried llc -march=aarch64 -run-pass=aarch64-prelegalizer-combiner foo.mir -o foo2.mirand got:

 bb.0 (%ir-block.1):
    liveins: $x0

    %0:_(p0) = COPY $x0
    %3:_(s32) = G_CONSTANT i32 0
    %2:_(<4 x s32>) = G_BUILD_VECTOR %3(s32), %3(s32), %3(s32), %3(s32)
    G_STORE %2(<4 x s32>), %0(p0) :: (store (<4 x s32>) into %ir.0)
    %4:_(s32) = G_LOAD %0(p0) :: (load (s32))
    $w0 = COPY %4(s32)
    RET_ReallyLR implicit $w0

@topperc
Copy link
Collaborator

topperc commented Jan 17, 2024

Looks like the (G_EXTRACT_ELT (G_LOAD)) -> G_LOAD combine inserts the scalar load at the extract element and not where the G_LOAD was. So the load is no longer before the store.

@tschuett
Copy link

bool CombinerHelper::matchCombineExtractedVectorLoad(MachineInstr &MI,

The G_EXTRACT_VECTOR_ELT is combined into a G_LOAD.

resistor added a commit to resistor/llvm-project that referenced this issue Feb 20, 2024
resistor added a commit to resistor/llvm-project that referenced this issue Feb 21, 2024
resistor added a commit that referenced this issue Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants