avr-rust / rust-legacy-fork

[deprecated; merged upstream] A fork of the Rust programming language with AVR support
Other
492 stars 14 forks source link

Multiplication miscompiles in a function with 6 parameters #157

Closed aykevl closed 4 years ago

aykevl commented 4 years ago

I found a rather strange miscompilation. IR:

target datalayout = "e-P1-p:16:8-i8:8-i16:8-i32:8-i64:8-f32:8-f64:8-n8-a:8"
target triple = "avr-atmel-none"

define i16 @sliceAppend(i16, i16, i16, i16, i16, i16) addrspace(1) {
  %d = mul i16 %0, %5
  ret i16 %d
}

Assembly (with llc -O2 -mcpu=atmega328p --filetype=obj test.ll):

00000000 <sliceAppend>:
   0:   8f 02           muls    r24, r31
   2:   20 2d           mov r18, r0
   4:   11 24           eor r1, r1
   6:   8e 9d           mul r24, r14
   8:   30 2d           mov r19, r0
   a:   41 2d           mov r20, r1
   c:   11 24           eor r1, r1
   e:   42 0f           add r20, r18
  10:   9e 02           muls    r25, r30
  12:   11 24           eor r1, r1
  14:   60 2d           mov r22, r0
  16:   64 0f           add r22, r20
  18:   66 0f           add r22, r22
  1a:   77 1f           adc r23, r23
  1c:   66 0f           add r22, r22
  1e:   77 1f           adc r23, r23
  20:   66 0f           add r22, r22
  22:   77 1f           adc r23, r23
  24:   66 0f           add r22, r22
  26:   77 1f           adc r23, r23
  28:   66 0f           add r22, r22
  2a:   77 1f           adc r23, r23
  2c:   66 0f           add r22, r22
  2e:   77 1f           adc r23, r23
  30:   66 0f           add r22, r22
  32:   77 1f           adc r23, r23
  34:   66 0f           add r22, r22
  36:   77 1f           adc r23, r23
  38:   83 2f           mov r24, r19
  3a:   99 27           eor r25, r25
  3c:   86 2b           or  r24, r22
  3e:   97 2b           or  r25, r23
  40:   08 95           ret

Look at that first instruction. It's multiplying r24 and r31. But r31 is not set anywhere in the code, and is not part of the passed-in parameters.

EDIT: simplified even further.

aykevl commented 4 years ago

I found the bug and made a patch, I'll submit it after running the tests.

aykevl commented 4 years ago

Fix: https://reviews.llvm.org/D74281

dylanmckay commented 4 years ago

I'm currently working on an upstreaming branch rather than a rebase, the fix which is now in LLVM master will flow to Rust on the next Rust LLVM update I suppose.

aykevl commented 4 years ago

Ah I guess this can be closed since it's merged upstream.