Expected either Y or Z register

[hercek]

The invalid assembly generated error seems to be back when compiling blink.

rustup run avr-toolchain xargo build --target avr-atmega328p --release
Compiling core v0.0.0 (file:///home/peter/rust/rust/src/libcore)
Expected either Y or Z register
UNREACHABLE executed at /home/peter/rust/rust/src/llvm/lib/Target/AVR/MCTargetDesc/AVRMCCodeEmitter.cpp:146!
error: Could not compile core.

[asheidan]

I got the same error message while compiling the blink project (though the error seems to be from libcore) but the cause was another file:

rustup run avr-toolchain xargo build --target avr-atmega328p --release
   Compiling core v0.0.0 (/Users/emieri/Documents/Source/avr-rust/rust/src/libcore)
Expected either Y or Z register
UNREACHABLE executed at ~/avr-rust/rust/src/llvm/lib/Target/AVR/MCTargetDesc/AVRMCCodeEmitter.cpp:146!
error: Could not compile `core`.

Please let me know if I can help out by providing more information.

[Vinc0682]

I have the same issue in combination with a message that wont go away after running xargo update or cargo update.

warning: Patch rustc-std-workspace-core v1.0.0 (/home/myusername/Downloads/rduino/rust/src/tools/rustc-std-workspace-core) was not used in the crate graph.
Check that the patched package version and available features are compatible
with the dependency requirements. If the patch has a different version from
what is locked in the Cargo.lock file, run cargo update to use the new
version. This may also occur with an optional dependency that is not enabled.
   Compiling core v0.0.0 (/home/myusername/Downloads/rduino/rust/src/libcore)
Expected either Y or Z register
UNREACHABLE executed at /home/myusername/Downloads/rduino/rust/src/llvm/lib/Target/AVR/MCTargetDesc/AVRMCCodeEmitter.cpp:146!
error: Could not compile core.

Caused by:
  process didn't exit successfully: rustc --crate-name core /home/myusername/Downloads/rduino/rust/src/libcore/lib.rs --color always --crate-type lib --emit=dep-info,link -C opt-level=3 -C metadata=d55c7e2141792a61 -C extra-filename=-d55c7e2141792a61 --out-dir /tmp/xargo.5TwVNc9HZZly/target/avr-atmega328p/release/deps --target avr-atmega328p -L dependency=/tmp/xargo.5TwVNc9HZZly/target/avr-atmega328p/release/deps -L dependency=/tmp/xargo.5TwVNc9HZZly/target/release/deps --sysroot /home/myusername/.xargo -Z force-unstable-if-unmarked (signal: 6, SIGABRT: process abort signal)
error: "cargo" "build" "--release" "--manifest-path" "/tmp/xargo.5TwVNc9HZZly/Cargo.toml" "--target" "avr-atmega328p" "-p" "core"` failed with exit code: Some(101)```

Stacktrace: 
`stack backtrace:
   0: error_chain::make_backtrace::hcdf05aaec9e819e0 (0x5617e25cc817)
   1: <error_chain::State as core::default::Default>::default::h69c20e25da695e17 (0x5617e25cc8a5)
   2: <std::process::Command as xargo::extensions::CommandExt>::run::h0942c6558146e76d (0x5617e2586daf)
   3: xargo::sysroot::build::h6514e4e6ffd86301 (0x5617e258fba8)
   4: xargo::sysroot::update::h137743eccc702b26 (0x5617e2593314)
   5: xargo::main::h402bd633f396879f (0x5617e25a0209)
   6: std::rt::lang_start::{{closure}}::h2e435953cbd00c95 (0x5617e257f6c2)
   7: std::rt::lang_start_internal::{{closure}}::h95abf94266177bce (0x5617e25eba42)
             at src/libstd/rt.rs:49
      std::panicking::try::do_call::h197f8ee7e1151bb3
             at src/libstd/panicking.rs:297
   8: __rust_maybe_catch_panic (0x5617e25f4309)
             at src/libpanic_unwind/lib.rs:92
   9: std::panicking::try::he1e9af636df1f58f (0x5617e25ec595)
             at src/libstd/panicking.rs:276
      std::panic::catch_unwind::hd1ca050dcb3c7db9
             at src/libstd/panic.rs:388
      std::rt::lang_start_internal::he9a94834a09ccf7f
             at src/libstd/rt.rs:48
  10: main (0x5617e25a0c91)
  11: __libc_start_main (0x7f1106a35b96)
  12: _start (0x5617e25744b9)
  13: <unknown> (0x0)
`

How to reproduce:

1. Clone this repository

2. Follow the installation instructions, I have changed the prefix in the configure step and not set avr-rust to my default toolchain

3. Clone the blink example

4. Follow the usage instructions with the adjusted path to the avr-rust src

[carlos4242]

I was about to raise a dupe...

Found the same issue myself.

[carlos4242]

This LLVM IR is failing to compile and crashing LLC.

; ModuleID = 'main_clean.ll'
source_filename = "main.ll"
target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.9"

@start2 = external local_unnamed_addr global i8*, align 8
@finish3 = external local_unnamed_addr global i8*, align 8
@__swift_reflection_version = linkonce_odr hidden constant i16 1
@llvm.used = appending global [1 x i8*] [i8* bitcast (i16* @__swift_reflection_version to i8*)], section "llvm.metadata"

define i32 @main(i32, i8** nocapture readnone) local_unnamed_addr #0 {
entry:
  tail call void @_TF3AVR11SetupSerialFT8baudRateVs6UInt16_T_(i16 -7936)
  %2 = load i64, i64* bitcast (i8** @start2 to i64*), align 8
  %3 = tail call i1 @_TIF3AVR5printFT7messageGSqGSPVs4Int8__10addNewlineSb_T_A0_()
  tail call void @_TF3AVR5printFT7messageGSqGSPVs4Int8__10addNewlineSb_T_(i64 %2, i1 %3)
  %4 = tail call i8* @_TF4mainau2i1Vs5Int16()
  %._value = bitcast i8* %4 to i16*
  %5 = load i16, i16* %._value, align 2
  %6 = tail call i8* @_TF4mainau2i2Vs5Int16()
  %._value1 = bitcast i8* %6 to i16*
  %7 = load i16, i16* %._value1, align 2
  %8 = add i16 %7, %5
  tail call void @_TF4main6expectFTVs5Int16S0__T_(i16 %8, i16 149)
  %9 = tail call i8* @_TF4mainau2i3Vs5Int16()
  %._value2 = bitcast i8* %9 to i16*
  %10 = load i16, i16* %._value2, align 2
  %11 = tail call i8* @_TF4mainau2i4Vs5Int16()
  %._value3 = bitcast i8* %11 to i16*
  %12 = load i16, i16* %._value3, align 2
  %13 = add i16 %12, %10
  tail call void @_TF4main6expectFTVs5Int16S0__T_(i16 %13, i16 108)
  %14 = tail call i8* @_TF4mainau2i5Vs5Int16()
  %._value4 = bitcast i8* %14 to i16*
  %15 = load i16, i16* %._value4, align 2
  %16 = tail call i8* @_TF4mainau2i6Vs5Int16()
  %._value5 = bitcast i8* %16 to i16*
  %17 = load i16, i16* %._value5, align 2
  %18 = add i16 %17, %15
  tail call void @_TF4main6expectFTVs5Int16S0__T_(i16 %18, i16 53)
  %19 = load i16, i16* %._value4, align 2
  %20 = load i16, i16* %._value5, align 2
  %21 = sub i16 %19, %20
  tail call void @_TF4main6expectFTVs5Int16S0__T_(i16 %21, i16 -39)
  %22 = load i16, i16* %._value5, align 2
  %23 = load i16, i16* %._value4, align 2
  %24 = sub i16 %22, %23
  tail call void @_TF4main6expectFTVs5Int16S0__T_(i16 %24, i16 39)
  %25 = load i16, i16* %._value, align 2
  %26 = load i16, i16* %._value1, align 2
  %27 = mul i16 %26, %25
  tail call void @_TF4main6expectFTVs5Int16S0__T_(i16 %27, i16 5130)
  %28 = load i16, i16* %._value, align 2
  %29 = load i16, i16* %._value1, align 2
  %30 = icmp eq i16 %29, 0
  br i1 %30, label %109, label %31

Compiled with llc main_clean_opt.ll -O3 -march=avr -mcpu=atmega328p -filetype=obj it crashes:

Expected either Y or Z register
UNREACHABLE executed at ../../lib/Target/AVR/MCTargetDesc/AVRMCCodeEmitter.cpp:146!
Stack dump:
0.	Program arguments: bin/llc main_clean_opt.ll -O3 -march=avr -mcpu=atmega328p -filetype=obj 
1.	Running pass 'Function Pass Manager' on module 'main_clean_opt.ll'.
2.	Running pass 'AVR Assembly Printer' on function '@main'
0  llc                      0x00000001114e519c llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 60
1  llc                      0x00000001114e5769 PrintStackTraceSignalHandler(void*) + 25
2  llc                      0x00000001114e1e3e llvm::sys::RunSignalHandlers() + 990
3  llc                      0x00000001114e6409 SignalHandler(int) + 505
4  libsystem_platform.dylib 0x00007fff60069b3d _sigtramp + 29
5  libsystem_platform.dylib 0x0000000116b38bc6 _sigtramp + 3064787110
6  libsystem_c.dylib        0x00007fff5ff271c9 abort + 127
7  llc                      0x0000000111388110 llvm::install_out_of_memory_new_handler() + 0
8  llc                      0x000000010f8cf2b7 llvm::AVRMCCodeEmitter::encodeMemri(llvm::MCInst const&, unsigned int, llvm::SmallVectorImpl<llvm::MCFixup>&, llvm::MCSubtargetInfo const&) const + 263
9  llc                      0x000000010f8d0365 llvm::AVRMCCodeEmitter::getBinaryCodeForInstr(llvm::MCInst const&, llvm::SmallVectorImpl<llvm::MCFixup>&, llvm::MCSubtargetInfo const&) const + 1957
10 llc                      0x000000010f8cfb7b llvm::AVRMCCodeEmitter::encodeInstruction(llvm::MCInst const&, llvm::raw_ostream&, llvm::SmallVectorImpl<llvm::MCFixup>&, llvm::MCSubtargetInfo const&) const + 187
11 llc                      0x00000001109caeb3 llvm::MCELFStreamer::EmitInstToData(llvm::MCInst const&, llvm::MCSubtargetInfo const&) + 211
12 llc                      0x0000000110a0716a llvm::MCObjectStreamer::EmitInstructionImpl(llvm::MCInst const&, llvm::MCSubtargetInfo const&) + 250
13 llc                      0x0000000110a07040 llvm::MCObjectStreamer::EmitInstruction(llvm::MCInst const&, llvm::MCSubtargetInfo const&, bool) + 80
14 llc                      0x000000010fd2567e llvm::AsmPrinter::EmitToStreamer(llvm::MCStreamer&, llvm::MCInst const&) + 78
15 llc                      0x000000010f8802a4 llvm::AVRAsmPrinter::EmitInstruction(llvm::MachineInstr const*) + 164
16 llc                      0x000000010fd2db63 llvm::AsmPrinter::EmitFunctionBody() + 4419
17 llc                      0x000000010f880458 llvm::AsmPrinter::runOnMachineFunction(llvm::MachineFunction&) + 56
18 llc                      0x0000000110126471 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) + 545
19 llc                      0x00000001107e6f67 llvm::FPPassManager::runOnFunction(llvm::Function&) + 503
20 llc                      0x00000001107e7635 llvm::FPPassManager::runOnModule(llvm::Module&) + 117
21 llc                      0x00000001107e819a (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) + 1546
22 llc                      0x00000001107e790e llvm::legacy::PassManagerImpl::run(llvm::Module&) + 366
23 llc                      0x00000001107e8cb1 llvm::legacy::PassManager::run(llvm::Module&) + 33
24 llc                      0x000000010eff383f compileModule(char**, llvm::LLVMContext&) + 24159
25 llc                      0x000000010efecd9f main + 5487
26 libdyld.dylib            0x00007fff5fe7eed9 start + 1
Abort trap: 6

Compiling to .s and using avr-gcc shows that the error is we aren't allowed to use LDD with the X register.

Compiled to assembly it produces this non compilable code:

        mov     r16, r24
        movw    r18, r10
        movw    r20, r12
        movw    r22, r14
        movw    r24, r8
        call    _TF3AVR5printFT7messageGSqGSPVs4Int8__10addNewlineSb_T_
        call    _TF4mainau2i1Vs5Int16
        movw    r26, r24
        ld      r16, X
        ldd     r17, X+1
        movw    r12, r26

ldd r17, X+1 is not valid AVR assembly I think? https://www.microchip.com/webdoc/avrassembler/avrassembler.wb_instruction_list.html

My suspicion is that commit 87c4dc90ec85cd7d107adfe66e3707346b3ad796 has caused a regression. I'll try to investigate after sleep.

[carlos4242]

This minimum llvm IR causes the crash:

define i32 @main(i32, i8** nocapture readnone) local_unnamed_addr {
entry:
  %2 = tail call i8* @globalVal()
  %._value = bitcast i8* %2 to i16*
  %3 = load i16, i16* %._value, align 2
  tail call void @expect(i16 %3, i16 149)
  ret i32 0
}

declare hidden void @expect(i16, i16) local_unnamed_addr
declare hidden i8* @globalVal() local_unnamed_addr

It produces this assembly:

	.text
	.file	"main_clean_opt.ll"
	.globl	main                    ; -- Begin function main
	.p2align	1
	.type	main,@function
main:                                   ; @main
; %bb.0:                                ; %entry
	call	globalVal
	movw	r26, r24
	ld	r24, X
	ldd	r25, X+1
	ldi	r22, 149
	ldi	r23, 0
	call	expect
	ldi	r22, 0
	ldi	r23, 0
	movw	r24, r22
	ret
.Lfunc_end0:
	.size	main, .Lfunc_end0-main
                                        ; -- End function
	.hidden	expect
	.hidden	globalVal

This is invalid and cannot be compiled by either gcc or llvm, it should be using Y or Z for indirect access, not X; outputting an object file instead of assembly causes the crash. I'll bugpoint and try to bisect which commit(s) introduced the issue later. Note: 87c4dc90ec85cd7d107adfe66e3707346b3ad796 has turned this into a crash but there were existing issues that needed resolution, so reverting that commit does not resolve the problem.

[hercek]

I do not know what is the syntax used by llvm but the proper assembly should have been:

ld    r24, X+    ; load byte pointed by X to r24 and postincrement X
ld    r25, X     ; load byte pointed by X to r25

These are valid instructions (ld instruction uses X register) and the message that register Y or Z should have been used is wrong.

Edit: Or maybe it really should have used Y or Z register because llvm authors decided that X will not be used in this context. The point is that all (X,Y,Z) are possible as for as at least ATmega23u4 and ATXmega128A4U are concerned.

Edit2: I see. They limited themselves to Y and Z registers so that they can use ldd r,Y+q and ldd r,Z+q variants. Even when it is not needed in this case.

[carlos4242]

That exactly what the compiler used to do before. This commit changed the behaviour: avr-rust/llvm@87c4dc9 The bug being fixed is that ld r24, X+ ld r25, X Is valid but changes X (I.e. r26/r27) which was causing issues to some people. This fix seems to have changed the emitted code to ld r24, X ldd r25, X+1 To avoid side effects. Which is invalid for X (but works for Y or Z) I’m not clear how the patch works but I think there are two broad approaches that seem sense: 1) leave the new code but change the register selection so that it will only ever select Y or Z. 2) revert to the old code, but find a way to tell the compiler the X register is “clobbered” after this operation and cannot be reused. It should act accordingly. I’m not really sure how to do either so I’m only helping with advice and unit tests until more experienced people can help pick it up!

…

On 10 Mar 2019, at 19:38, hercek ***@***.***> wrote: I do not know what is the syntax used by llvm but the proper assembly should have been: ld r24, X+ ; load byte pointed by X to r24 and postincrement X ld r25, X ; load byte pointed by X to r25 These are valid instructions (ld instruction uses X register) and the message that register Y or Z should have been used is wrong. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[carlos4242]

Ran bugpoint to get a minimum test case:

define i32 @main(i32, i8** nocapture readnone) local_unnamed_addr {
entry:
  %2 = load i16, i16* undef, align 2
  tail call void @expect(i16 %2, i16 149)
  ret i32 0
}

declare hidden void @expect(i16, i16) local_unnamed_addr
declare hidden i8* @globalVal() local_unnamed_addr

[carlos4242]

OK have a hack patch you guys can use to get going for emergency purposes:
avr-rust/llvm#11

The patch is meant to limit the selection of the LDWRdPtr pseudo instruction to the Y and Z registers, hopefully eliminating the crash for you guys (and me and my customers).

But it should be considered only if you need to get going now. The better solution will be to wait until more experienced developers (@dylanmckay @shepmaster @brainlag) have had a chance to look at the issue and come up with the correct solution (whatever that is).

Here is the assembly produced when the above minimum test case is compiled:

main:                                   ; @main
; %bb.0:                                ; %entry
	ld	r24, Z
	ldd	r25, Z+1
	ldi	r22, 149
	ldi	r23, 0
	call	expect
	ldi	r22, 0
	ldi	r23, 0
	movw	r24, r22
	ret

...and it compiles with -filetype=obj so it should fix you guys' issue.

But...

This patch breaks two unit tests, and really needs more experienced developers to check it.

    LLVM :: CodeGen/AVR/high-pressure-on-ptrregs.ll
    LLVM :: CodeGen/AVR/load.ll

The second one is incompatible with the patch because it expects the X register to be used, that might just mean if my fix is valid then that unit test needs altering accordingly. The first one looks more serious because it suggests that the patch increases register pressure in some circumstances so it could cause some other more complex compilations to crash.

Hope this helps anyway.

Carl

[hercek]

High register pressure should not lead to a crash. Variables can be always spilled to memory if there is enough registers to perform one binary operation in them (i.e. for two operands and a memory pointer). AVR has plenty enough registers for that even when some registers are dedicated to e.g. stack frame. The generated code would be slow but I prefer a slow code which works to a quick code which cannot be generated at all.

Anyway that was just a rant. I wanted to ask two things:

1. Where one can read documents identified as PR39553 or PR13375 etc.
2. Where does this code (from AVRInstrInfo.td) define that the value of $ptrreg is incremented/modified/destroyed?

let canFoldAsLoad = 1,
isReMaterializable = 1 in
{
  // LDW Rd+1:Rd, P
  // Expands to:
  // ld Rd,   P+
  // ld Rd+1, P
  let Constraints = "@earlyclobber $reg" in
  def LDWRdPtr : Pseudo<(outs DREGS:$reg),
                        (ins PTRREGS:$ptrreg),
                        "ldw\t$reg, $ptrreg",
                        [(set i16:$reg, (load i16:$ptrreg))]>,
                 Requires<[HasSRAM]>;
}

[carlos4242]

I’m sure you’re right generally. My guess is that because these are special registers, there are certain times where they have to be used and stack spilling isn’t an option, for example the frame pointer. I don’t have a unit test that will cause the run out of registers crash so I can’t debug what is going on but my guess would be that there is specific code to throw an error when these specific register shortages occur. You’re right that the block below does not indicate that the value of $ptrreg is incremented/modified/destroyed, because it no longer is. Since Keshav's patch was included in avr-rust, in the avr-rust-2019-01-18 branch, the ptr register is no longer clobbered because the pseudo code expansion now uses LDD. But his patch forgot to take account of the fact that the X register is not valid for this code, that is what my small patch corrected. But my patch has now sometimes caused running out of registers. Does that make sense? What i’m looking at with Dylan and the LLVM developers is whether it’s possible to revert Keshav’s patch, go back to the old code but change the pseudo function definition to define that the ptrreg is clobbered. I don’t currently know how to do that as I’m a novice developer when it comes to LLVM*. What do you think? (*I have over 25 years of professional development on other platforms/languages.)

…

On 13 Mar 2019, at 09:14, hercek ***@***.***> wrote: High register pressure should not lead to a crash. Variables can be always spilled to memory if there is enough registers to perform one binary operation in them (i.e. for two operands and a memory pointer). AVR has plenty enough registers for that even when some registers are dedicated to e.g. stack frame. The generated code would be slow but I prefer a slow code which works to a quick code which cannot be generated at all. Anyway that was just a rant. I wanted to ask two things: Where one can read documents identified as PR39553 or PR13375 etc. Where does this code (from AVRInstrInfo.td) define that the value of $ptrreg is incremented/modified/destroyed? let canFoldAsLoad = 1, isReMaterializable = 1 in { // LDW Rd+1:Rd, P // Expands to: // ld Rd, P+ // ld Rd+1, P let Constraints = ***@***.*** $reg" in def LDWRdPtr : Pseudo<(outs DREGS:$reg), (ins PTRREGS:$ptrreg), "ldw\t$reg, $ptrreg", [(set i16:$reg, (load i16:$ptrreg))]>, Requires<[HasSRAM]>; } — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#128 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADP6WsiTDs7R6YEw3K0VtaYJvArcdLzuks5vWMFwgaJpZM4aZAlA>.

[hercek]

Well, I think that the best option is to divide the LDWRdPtr into two versions. One will use displacements only and work with Y and Z only. The second one will use memory indexed mode and work with X, Y, and Z.

The problem is that I never worked with llvm as well and I do not know how to indicate PRE_DEC and POST_INC MemIndexedMode. These modes are used in ARM. The inspiration can be taken from pre_store, post_store fragments of ARM pseudo-instructions for pattern matching indexed stores.

But I'm OK with a working solution. It does not need to be the best one.

[hercek]

I tried to compile blinky with carlos4242's patch. For a change, libcore cannot be compiled because llvm runs out of registers.