assert! and assert_eq! generate different assembly

[hellow554]

Concidering this simple code

pub fn foo1(a: u32, b: u32) -> bool {
    assert!(a == b);
    true
}

pub fn foo2(a: u32, b: u32) -> bool {
    assert_eq!(a, b);
    true
}

The generate very different assembly

example::foo1:
  push rax
  cmp edi, esi
  jne .LBB7_1
  mov al, 1
  pop rcx
  ret
.LBB7_1:
// panicing

example::foo2:
  sub rsp, 104
  mov dword ptr [rsp], edi
  mov dword ptr [rsp + 4], esi
  mov rax, rsp
  mov qword ptr [rsp + 8], rax
  lea rax, [rsp + 4]
  mov qword ptr [rsp + 16], rax
  cmp edi, esi
  jne .LBB8_1
  mov al, 1
  add rsp, 104
  ret
.LBB8_1: 
// panicing

There is a difference regarding the parameter formatting in the "assert_false" branch LBB7_1 and LBB8_1, but what's even worse is, that the assert itself does differ and I don't see a reason why.

[cramertj]

These two things give different output. With assert!(x == y), you get

thread 'main' panicked at 'assertion failed: x == y', src/main.rs:5:5

while assert_eq!(x, y) gives

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `0`,
 right: `5`', src/main.rs:5:5

[cuviper]

It's implemented like this:

macro_rules! assert_eq {
    ($left:expr, $right:expr) => ({
        match (&$left, &$right) {
            (left_val, right_val) => {
                if !(*left_val == *right_val) {
                    panic!(r#"assertion failed: `(left == right)`
  left: `{:?}`,
 right: `{:?}`"#, left_val, right_val)
                }
            }
        }
    });

Note how the match is used to save the value of each expr, so they can be printed in the panic message. Whereas the plain assert! doesn't need to keep any part of its expression alive.

In your case it's still using the original register for the actual comparison, cmp edi, esi, but that extra stack preparation is only used in the panicking case. I wonder if std::intrinsics::unlikely would be enough of a hint to LLVM, so it might decide to delay that setup...

[hellow554]

As an update to this:

1.30.0:

example::foo2:
  sub rsp, 104
  mov dword ptr [rsp], edi
  mov dword ptr [rsp + 4], esi
  mov rax, rsp
  mov qword ptr [rsp + 8], rax
  lea rax, [rsp + 4]
  mov qword ptr [rsp + 16], rax
  cmp edi, esi
  jne .LBB8_1
  mov al, 1
  add rsp, 104
  ret

1.36.0

example::foo2:
        sub     rsp, 104
        mov     dword ptr [rsp], edi
        mov     dword ptr [rsp + 4], esi
        cmp     edi, esi
        jne     .LBB9_1
        mov     al, 1
        add     rsp, 104
        ret

huge improvement. A few things could be done better, e.g. the sub rsp, 104 could be moved after the jne, but I think this has to be done according to #57815 (comment) but I'm not entirely sure. The two movs are done, because assert_eq matches the references but all in all, looks okay-ish to me.

Should we close this?

[tesuji]

What about a codegen regression test?

[hellow554]

On current stable (1.52) this looks promising, but still not from perfect:

assert!(a == b)

example::foo1:
  cmp edi, esi
  jne .LBB3_1
  mov al, 1
  ret
.LBB3_1:
// panic

assert_eq!(a, b)

example::foo2:
  sub rsp, 56
  mov dword ptr [rsp], edi
  mov dword ptr [rsp + 4], esi
  cmp edi, esi
  jne .LBB4_1
  mov al, 1
  add rsp, 56
  ret
.LBB4_1:
// panic