Possible miscompilation on armv7 FreeBSD 14.1

[clausecker]

Greetings,

While investigating a spurious test suite failure of the simdutf test suite, we found that the failure would only reproduce under specific circumstances:

• recent llvm version (such as, the llvm 18.1.5 shipped with FreeBSD 14.1, but also a very recent devel snapshot)
• build for armv7 FreeBSD
• build with -fno-strict-aliasing, code works fine with -fstrict-aliasing

The result of the miscompilation is that wrong code is generated, leading to unit test failure.

A reduced example can be found in simdutf/simdutf#581 (comment).

[llvmbot]

@llvm/issue-subscribers-backend-arm

Author: Robert Clausecker (clausecker)

Greetings,

While investigating a spurious test suite failure of the simdutf test suite, we found that the failure would only reproduce under specific circumstances:

• recent llvm version (such as, the llvm 18.1.5 shipped with FreeBSD 14.1, but also a very recent devel snapshot)
• build for armv7 FreeBSD
• build with -fno-strict-aliasing, code works fine with -fstrict-aliasing

The result of the miscompilation is that wrong code is generated, leading to unit test failure.

A reduced example can be found in simdutf/simdutf#581 (comment).

[pauldreik]

@dtcxzyw what information is needed to make this complete?

[antoniofrighetto]

@dtcxzyw Haven't tried clang on trunk yet, but I managed to reproduce this with clang 18.1.5 targeting armv7 on FreeBSD.

[dtcxzyw]

@dtcxzyw what information is needed to make this complete?

Can you provide a single-file reproducer? A godbolt link would be better :)

[dtcxzyw]

@dtcxzyw what information is needed to make this complete?

Can you provide a single-file reproducer? A godbolt link would be better :)

Oh sorry. I see that in the issue comment.

[antoniofrighetto]

The IR looks fine. I think the miscompilation lies immediately before the store utf16[i + 0] = 0xd800;, when saving the value of utf16[i + 0] in old_W1. Here is how the correct codegen looks like:

bls    std::__1::allocator<char16_t>>::operator[]
add    r1, r0, r7
mov    r2, r5
ldrh   r8, [r1]
mov    r1, #55296
strh   r1, [r0, r7]!

utf16[0] is saved in r8 before storing 0xd800 on it. The load however seems to have been incorrectly moved around with -fno-strict-aliasing, leading to store 0xd800 without saving its original value:

bls    std::__1::allocator<char16_t>>::operator[]
mov    r1, r0
mov    r2, #55296
strh   r2, [r1, r7]!

Culprit seems to be machine code sinking, MIR before machine-sink:

bb.7 (%ir-block.24):
  %27:gpr = ADDrr %5:gpr, %3:gpr, 14, $noreg, $noreg
  %7:gpr = LDRH killed %27:gpr, $noreg, 0, 14, $noreg :: (load (s16) from %ir.scevgep5)
  %8:gpr = ADDri %6:gpr, 1, 14, $noreg, $noreg
  CMPrr %4:gpr, %8:gpr, 14, $noreg, implicit-def $cpsr
  Bcc %bb.9, 8, $cpsr
  B %bb.8

bb.9 (%ir-block.29):
  early-clobber %29:gpr = STRH_PRE %28:gpr, %5:gpr(tied-def 0), %3:gpr, 0, 14, $noreg

The buffer value is correctly loaded and saved before the STRH_PRE of %28:gpr = MOVi 55296, 14, $noreg, $noreg. After the pass, the LDRH seems to have been sunk too down in a few successor basic block later, breaking its dependency constraint:

bb.7 (%ir-block.24):
  %8:gpr = ADDri %6:gpr, 1, 14, $noreg, $noreg
  CMPrr %4:gpr, %8:gpr, 14, $noreg, implicit-def $cpsr
  Bcc %bb.9, 8, $cpsr
  B %bb.8

bb.9 (%ir-block.29):
  early-clobber %29:gpr = STRH_PRE %28:gpr, %5:gpr(tied-def 0), %3:gpr, 0, 14, $noreg
  ; ...
  Bcc %bb.11, 3, $cpsr
  B %bb.10

bb.11 (%ir-block.39):
  %27:gpr = ADDrr %5:gpr, %3:gpr, 14, $noreg, $noreg
  %7:gpr = LDRH %27:gpr, $noreg, 0, 14, $noreg :: (load (s16) from %ir.scevgep5)

The original value has been overwritten by the time we load utf16[0].