1.39.0 new tests sync::atomic::Atomic*::fetch_{min,max} fail on mips*

[infinity0]

Tests were ran on 64- and 32-bit little-endian, Debian dropped support for big-endian a few months ago.

• https://buildd.debian.org/status/fetch.php?pkg=rustc&arch=mipsel&ver=1.39.0%2Bdfsg1-3&stamp=1575841021&raw=0
• https://buildd.debian.org/status/fetch.php?pkg=rustc&arch=mipsel&ver=1.38.0%2Bdfsg1-2&stamp=1575039326&raw=0

One example:

---- num/mod.rs - sync::atomic::AtomicI16::fetch_max (line 49) stdout ----
LLVM ERROR: Cannot select: 0x561489c8: i32,ch = AtomicLoadMax<(volatile load store monotonic 2 on %ir.0)> 0x56149160, 0x56148f20, 0x561487d0
  0x56148f20: i32,ch = CopyFromReg 0x55fdd1d4, Register:i32 %0
    0x56148938: i32 = Register %0
  0x561487d0: i32,ch = CopyFromReg 0x55fdd1d4, Register:i32 %3
    0x561490d0: i32 = Register %3
In function: _ZN4core4sync6atomic10atomic_max17hdde194b3ab9a7f02E
Couldn't compile the test.

[jrtc27]

Upstream LLVM bug (happens for both i32 and i64, though only i32 is allowed to be generated by the Clang builtin) as the atomic RMW is not expanded and not handled by the backend:

% printf 'define i32 @max(i32 *%%ptr, i32 %%val) {\n %%1 = atomicrmw umax i32 *%%ptr, i32 %%val monotonic\n ret i32 %%1\n}' | llc -mtriple mips64
	.text
	.section	.mdebug.abi64,"",@progbits
	.nan	legacy
	.text
	.file	"<stdin>"
LLVM ERROR: Cannot select: t6: i32,ch = AtomicLoadUMax<(load store monotonic 4 on %ir.ptr)> t0, t2, t5
  t2: i64,ch = CopyFromReg t0, Register:i64 %0
    t1: i64 = Register %0
  t5: i32 = truncate t4
    t4: i64,ch = CopyFromReg t0, Register:i64 %1
      t3: i64 = Register %1
In function: max

% printf 'define i64 @max(i64 *%%ptr, i64 %%val) {\n %%1 = atomicrmw umax i64 *%%ptr, i64 %%val monotonic\n ret i64 %%1\n}' | llc -mtriple mips64
	.text
	.section	.mdebug.abi64,"",@progbits
	.nan	legacy
	.text
	.file	"<stdin>"
LLVM ERROR: Cannot select: t5: i64,ch = AtomicLoadUMax<(load store monotonic 8 on %ir.ptr)> t0, t2, t4
  t2: i64,ch = CopyFromReg t0, Register:i64 %0
    t1: i64 = Register %0
  t4: i64,ch = CopyFromReg t0, Register:i64 %1
    t3: i64 = Register %1
In function: max

Can also be triggered directly from Clang with:

% echo 'unsigned int max(unsigned int *ptr, unsigned int val) { return __atomic_fetch_max(ptr, val, __ATOMIC_RELAXED); }' | cheri-llvm clang -target mips64 -x c - -o - -S
	.text
	.abicalls
	.section	.mdebug.abi64,"",@progbits
	.nan	legacy
	.text
	.file	"-"
fatal error: error in backend: Cannot select: t22: i32,ch = AtomicLoadUMax<(load store monotonic 4 on %ir.0)> t21:1, t16, t21
  t16: i64,ch = load<(dereferenceable load 8 from %ir.ptr.addr)> t15, FrameIndex:i64<0>, undef:i64
    t10: i64 = FrameIndex<0>
    t12: i64 = undef
  t21: i32,ch = load<(dereferenceable load 4 from %ir..atomictmp)> t20, FrameIndex:i64<2>, undef:i64
    t18: i64 = FrameIndex<2>
    t12: i64 = undef
In function: max

(which generates the same IR at -O2 as my hand-written example, albeit with some extra debug metadata)

MipsExpandPseudo needs to learn about these (and the various pseudo ISDs added to its TableGen files). I'll see if I can have a look soon.

[jrtc27]

Somebody already worked on this https://reviews.llvm.org/D71028. Should be merged any moment, so Debian can backport it if desired.

[infinity0]

@jrtc27 sylvestre the Debian LLVM maintainer reports that applying this patch directly on top of LLVM 9 causes this build failure:

/build/llvm-toolchain-9-9.0.1/llvm/lib/Target/Mips/MipsExpandPseudo.cpp: In member function 'bool {anonymous}::MipsExpandPseudo::expandAtomicBinOp(llvm::MachineBasicBlock&, llvm::MachineBasicBlock::iterator, llvm::MachineBasicBlock::iterator&, unsigned int)':
/build/llvm-toolchain-9-9.0.1/llvm/lib/Target/Mips/MipsExpandPseudo.cpp:740:21: error: operands to ?: have different types 'unsigned int' and 'llvm::Register'
         (Size == 8) ? STI->getRegisterInfo()->getSubReg(Scratch2, Mips::sub_32)
~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                     : Scratch2;
                     ~~~~~~~~~~

[jrtc27]

Replace all instances of Register with unsigned in the patch (even in the context parts of the diff, as that's currently fuzz). 9 branched before that mass-refactoring happened.

[infinity0]

The LLVM backport worked to fix the tests in Debian. There is nothing to do on the rust side, so closing this bug report.