-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expected either Y or Z register #128
Comments
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:
Please let me know if I can help out by providing more information. |
I have the same issue in combination with a message that wont go away after running
|
I was about to raise a dupe... Found the same issue myself. |
This LLVM IR is failing to compile and crashing LLC.
Compiled with
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:
My suspicion is that commit |
This minimum llvm IR causes the crash:
It produces this assembly:
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: |
I do not know what is the syntax used by llvm but the proper assembly should have been:
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 |
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.
|
Ran bugpoint to get a minimum test case:
|
OK have a hack patch you guys can use to get going for emergency purposes: 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:
...and it compiles with But... This patch breaks two unit tests, and really needs more experienced developers to check it.
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 |
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:
|
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>.
|
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 But I'm OK with a working solution. It does not need to be the best one. |
I tried to compile blinky with carlos4242's patch. For a change, libcore cannot be compiled because llvm runs out of registers. |
Good approach. There are already multiple similar pseudo instructions like this. I'm really new to llvm, I think they each have a pattern to match and which one is selected depends on matching the pattern on the selectionDAG. So I guess the ideal would be a version of the instruction definitions that said "if you have Y or Z available, you can use this version, if not, use this version but the X will be clobbered". I have no idea how to write this. :) I think the best fix is to find out how to make the second version, which may be to do with Tied registers, I put a question to llvm-dev but haven't go an answer. |
p.s. On the subject of register pressure, I think there's also a sort of "underlying bug" that there should be no circumstances where you're forced to use one of the pointer registers but it was working before the patch so I guess that bug is lower priority. |
Well, even if register allocator thinks that it needs to use some particular register which is taken by a variable then I do not understand why it does not spill the variable into memory. The inability to spill looks like a bug. Anyway my modified version of the code generator with carlos4242's patch can compile
libcore does not compile as a consequence of that. |
The dreaded "ran out of registers" error again. I did some searching and came across PR18825 from the ARM backend
I tried compiling @hercek's IR file with It generated this assembly: aaa: ; @aaa
; %bb.0: ; %start
ld r18, Z
ldd r19, Z+1
movw r30, r24
ld r24, Z
ldd r25, Z+1
movw r30, r24
ldd r22, Z+2
ldd r23, Z+3
call _ZNE
.Lfunc_end0:
.size aaa, .Lfunc_end0-aaa
; -- End function
; Declaring this symbol tells the CRT that it should
;copy all variables from program memory to RAM on startup
.globl __do_copy_data
; Declaring this symbol tells the CRT that it should
;clear the zeroed data section on startup
.globl __do_clear_bss In that bug, Chris B created a patch. He describes it here:
The patch can be found here llvm-mirror/llvm@01d8611 If relaxing the register coalescer for AVR stops the register allocator choking on "ran out of registers", we should be able to add a hook to our target, just like ARM does in that same patch, to avoid coalescing for some sets of AVR programs. |
It looks to me that the only reasonable option is to disable register coalescing on Maybe it can be done more easily than by adding an The point is that Btw, the register allocator state when the error happens is here:
The problem is the instruction at index |
Since nobody answered I assume that my proposal for fix is good enough. Here is a patch witch fixes the LDWRdPtr instruction (as proposed by carlos4242) and the coalescing of PTRDISPREGS class (as roughly porposed by dylanmckay). It makes it possible to compile core library as it is present in this rust branch for AVR. From a31cb28eead215557746c515a59fc09af563c8d6 Mon Sep 17 00:00:00 2001
From: Peter Hercek <[email protected]>
Date: Sat, 13 Apr 2019 19:39:57 +0200
Subject: [PATCH] [AVR] Fix LDWRdPtr register class to PTRDISPREGS and disable
coalescing on it
LDWRdPtr instruction uses an immediate displacement and therefore supports
only PTRDISPREGS class and not the bigger PTRREGS class. Fixing the class
prevents the 'Expected either Y or Z class' compiler crash.
But the class fix leads to another compiler crash: register allocator runs
out of registers.
Memory access with a pointer in a register and a displacement is possible
only for registers in PTRDISPREGS class. This class has only two members: Y,Z.
If a stack frame is used then Y register is dedicated to it. This leaves only
register Z for holding pointers to memory.
If we would allow register coalescing on PTRDISPREGS class then register
allocator can lock Z register to some virtual register. Lager instructions
requiring a memory acces then fail during the register allocation phase since
there is no available register to hold a pointer if Y register was already
taken for a stack frame. This patch prevents it by keeping Z register
spillable. It does it by not allowing coalescer to lock it.
---
lib/Target/AVR/AVRInstrInfo.td | 6 +++---
lib/Target/AVR/AVRRegisterInfo.cpp | 10 ++++++++++
lib/Target/AVR/AVRRegisterInfo.h | 9 +++++++++
3 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/lib/Target/AVR/AVRInstrInfo.td b/lib/Target/AVR/AVRInstrInfo.td
index 5720af7d8df..3ef7fb8da4e 100644
--- a/lib/Target/AVR/AVRInstrInfo.td
+++ b/lib/Target/AVR/AVRInstrInfo.td
@@ -1151,11 +1151,11 @@ isReMaterializable = 1 in
// LDW Rd+1:Rd, P
//
// Expands to:
- // ld Rd, P+
- // ld Rd+1, P
+ // ld Rd, P
+ // ld Rd+1, P+1
let Constraints = "@earlyclobber $reg" in
def LDWRdPtr : Pseudo<(outs DREGS:$reg),
- (ins PTRREGS:$ptrreg),
+ (ins PTRDISPREGS:$ptrreg),
"ldw\t$reg, $ptrreg",
[(set i16:$reg, (load i16:$ptrreg))]>,
Requires<[HasSRAM]>;
diff --git a/lib/Target/AVR/AVRRegisterInfo.cpp b/lib/Target/AVR/AVRRegisterInfo.cpp
index 808a85e459c..f31bd9970e9 100644
--- a/lib/Target/AVR/AVRRegisterInfo.cpp
+++ b/lib/Target/AVR/AVRRegisterInfo.cpp
@@ -273,4 +273,14 @@ void AVRRegisterInfo::splitReg(unsigned Reg,
HiReg = getSubReg(Reg, AVR::sub_hi);
}
+bool AVRRegisterInfo::shouldCoalesce(MachineInstr * /*MI*/,
+ const TargetRegisterClass * /*SrcRC*/,
+ unsigned /*SubReg*/,
+ const TargetRegisterClass * /*DstRC*/,
+ unsigned /*DstSubReg*/,
+ const TargetRegisterClass *NewRC,
+ LiveIntervals & /*LIS*/) const {
+ return NewRC != &AVR::PTRDISPREGSRegClass;
+}
+
} // end of namespace llvm
diff --git a/lib/Target/AVR/AVRRegisterInfo.h b/lib/Target/AVR/AVRRegisterInfo.h
index 104b336b9c4..ae252c79d45 100644
--- a/lib/Target/AVR/AVRRegisterInfo.h
+++ b/lib/Target/AVR/AVRRegisterInfo.h
@@ -56,6 +56,15 @@ public:
return true;
}
+ /// SrcRC and DstRC will be morphed into NewRC if this returns true
+ bool shouldCoalesce(MachineInstr *MI,
+ const TargetRegisterClass *SrcRC,
+ unsigned SubReg,
+ const TargetRegisterClass *DstRC,
+ unsigned DstSubReg,
+ const TargetRegisterClass *NewRC,
+ LiveIntervals &LIS) const override;
+
};
} // end namespace llvm
--
2.21.0 It is not an ideal solution from the performance point of view but significantly better than a crashing compiler. |
BTW: libcore can be now compiled without DREGS_WITHOUT_Z_WORKAROUND after my patch. I wanted to try it since I do not understand why it should be needed when Z can be spilled now. Also if I understand the meaning of earclyclobber well then there is a disagreement between LDWRdPtr being marked as earlycloober and the corresponding expansion template specialization which checks and works around the src and dst registers being the same. Either earlyclobber is not needed or the src and dst check for being the same is not needed. Comments? |
Here's a reduced testcase from the updated codebase: ; ModuleID = 'bugpoint-reduced-simplified.bc'
source_filename = "core.37hoedwx-cgu.0"
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-unknown-unknown"
%num__flt2dec__Part = type { [0 x i8], i8, [4 x i8] }
%num__flt2dec__decoder__FullDecoded = type { [26 x i8], i8, [0 x i8] }
%num__flt2dec__Formatted = type { [0 x i8], { [0 x i8]*, i16 }, [0 x i8], { [0 x %num__flt2dec__Part]*, i16 }, [0 x i8] }
%num__flt2dec__Part__Copy = type { [1 x i8], { [0 x i8]*, i16 }, [0 x i8] }
@anon.4c2386cf7531a237a70031c52b819347.56 = external dso_local unnamed_addr constant <{ [1 x i8] }>, align 1
@anon.4c2386cf7531a237a70031c52b819347.57 = external dso_local unnamed_addr constant <{ [3 x i8] }>, align 1
@anon.4c2386cf7531a237a70031c52b819347.63 = external dso_local unnamed_addr constant <{ [3 x i8] }>, align 1
@anon.4c2386cf7531a237a70031c52b819347.64 = external dso_local unnamed_addr constant <{ [3 x i8] }>, align 1
declare dso_local fastcc { [0 x %num__flt2dec__Part]*, i16 } @core__slice__Index([0 x %num__flt2dec__Part]*, i16, i16) unnamed_addr addrspace(1) #0
declare i32 @rust_eh_personality(...) unnamed_addr addrspace(1) #0
declare void @_ZN4core3num7flt2dec7decoder6decode17h56dc0fcd87a4559cE({ [0 x i8], i8, [0 x i8], %num__flt2dec__decoder__FullDecoded, [0 x i8] }*, double) unnamed_addr addrspace(1) #0
declare { [0 x i8]*, i16 } @_ZN4core3num7flt2dec14determine_sign17h04b9993823707283E(i8, %num__flt2dec__decoder__FullDecoded*, i1) unnamed_addr addrspace(1) #0
define void @_ZN4core3num7flt2dec19to_shortest_exp_str17h47df2ff6a62ba60cE(%num__flt2dec__Formatted*, double, i8, i16, i16, i1, [0 x i8]*, i16, [0 x %num__flt2dec__Part]*, i16) unnamed_addr addrspace(1) #0 personality i32 (...) addrspace(1)* @rust_eh_personality {
start:
%10 = icmp ult i16 %9, 6
br i1 %10, label %bb4, label %bb5
bb4: ; preds = %start
unreachable
bb5: ; preds = %start
call addrspace(1) void @_ZN4core3num7flt2dec7decoder6decode17h56dc0fcd87a4559cE({ [0 x i8], i8, [0 x i8], %num__flt2dec__decoder__FullDecoded, [0 x i8] }* noalias nocapture nonnull sret dereferenceable(28) null, double %1)
%11 = call addrspace(1) { [0 x i8]*, i16 } @_ZN4core3num7flt2dec14determine_sign17h04b9993823707283E(i8 %2, %num__flt2dec__decoder__FullDecoded* noalias nonnull readonly align 1 dereferenceable(27) undef, i1 zeroext undef)
switch i2 undef, label %bb13 [
i2 0, label %bb15
i2 1, label %bb18
i2 -2, label %bb30
i2 -1, label %bb32
]
bb13: ; preds = %bb5
unreachable
bb15: ; preds = %bb5
%12 = tail call fastcc addrspace(1) { [0 x %num__flt2dec__Part]*, i16 } @core__slice__Index([0 x %num__flt2dec__Part]* noalias nonnull readonly align 1 %8, i16 %9, i16 1)
%13 = getelementptr inbounds %num__flt2dec__Formatted, %num__flt2dec__Formatted* %0, i16 0, i32 3, i32 1
store i16 0, i16* %13, align 1
ret void
bb18: ; preds = %bb5
%14 = bitcast [0 x %num__flt2dec__Part]* %8 to %num__flt2dec__Part__Copy*
%15 = getelementptr inbounds %num__flt2dec__Part__Copy, %num__flt2dec__Part__Copy* %14, i16 0, i32 1, i32 0
store [0 x i8]* bitcast (<{ [3 x i8] }>* @anon.4c2386cf7531a237a70031c52b819347.57 to [0 x i8]*), [0 x i8]** %15, align 1
unreachable
bb30: ; preds = %bb5
%16 = icmp slt i16 %3, 1
%17 = icmp sgt i16 %4, 0
%_61.0 = and i1 %16, %17
%. = select i1 %5, [0 x i8]* bitcast (<{ [3 x i8] }>* @anon.4c2386cf7531a237a70031c52b819347.63 to [0 x i8]*), [0 x i8]* bitcast (<{ [3 x i8] }>* @anon.4c2386cf7531a237a70031c52b819347.64 to [0 x i8]*)
%_60.sroa.5.0 = select i1 %_61.0, [0 x i8]* bitcast (<{ [1 x i8] }>* @anon.4c2386cf7531a237a70031c52b819347.56 to [0 x i8]*), [0 x i8]* %.
%_60.sroa.7.0 = select i1 %_61.0, i16 1, i16 3
store [0 x i8]* %_60.sroa.5.0, [0 x i8]** undef, align 1
%_60.sroa.7.0..sroa_idx = getelementptr inbounds [0 x %num__flt2dec__Part], [0 x %num__flt2dec__Part]* %8, i16 0, i16 0, i32 0, i16 3
%_60.sroa.7.0..sroa_cast52 = bitcast i8* %_60.sroa.7.0..sroa_idx to i16*
store i16 %_60.sroa.7.0, i16* %_60.sroa.7.0..sroa_cast52, align 1
unreachable
bb32: ; preds = %bb5
unreachable
}
attributes #0 = { "target-cpu"="atmega328p" }
!llvm.module.flags = !{!0}
!0 = !{i32 2, !"Debug Info Version", i32 3} It fails to compile, even with @hercek's patch, but does compile with
|
The AVR backend was written a long time ago (almost 10 years ago?) and a lot has changed in llvm but the AVR backend was never updated to these changes. This could be a bug in AVR, or a workaround for a specific bug in LLVM which doesn't exist anymore, or a workaround for a bug which still exists. Nobody really knows. If you think it is wrong you have to figure it out if this really the case. I don't think there is anyone around who can answer you why this is like it is. |
shepmaster: diff --git a/lib/Target/AVR/AVRInstrInfo.td b/lib/Target/AVR/AVRInstrInfo.td
index 3ef7fb8da4e..6d391abc0fd 100644
--- a/lib/Target/AVR/AVRInstrInfo.td
+++ b/lib/Target/AVR/AVRInstrInfo.td
@@ -1222,7 +1222,7 @@ isReMaterializable = 1 in
// ldd Rd, P+q
// ldd Rd+1, P+q+1
let Constraints = "@earlyclobber $dst" in
- def LDDWRdPtrQ : Pseudo<(outs DREGS_WITHOUT_Z_WORKAROUND:$dst),
+ def LDDWRdPtrQ : Pseudo<(outs DREGS:$dst),
(ins memri:$memri),
"lddw\t$dst, $memri",
[(set i16:$dst, (load addr:$memri))]>,
diff --git a/lib/Target/AVR/AVRRegisterInfo.td b/lib/Target/AVR/AVRRegisterInfo.td
index d55252bcac4..8162f12052b 100644
--- a/lib/Target/AVR/AVRRegisterInfo.td
+++ b/lib/Target/AVR/AVRRegisterInfo.td
@@ -157,26 +157,6 @@ def DREGS : RegisterClass<"AVR", [i16], 8,
R9R8, R7R6, R5R4, R3R2, R1R0
)>;
-// The 16-bit DREGS register class, excluding the Z pointer register.
-//
-// This is used by instructions which cause high pointer register
-// contention which leads to an assertion in the register allocator.
-//
-// There is no technical reason why instructions that use this class
-// cannot use Z; it's simply a workaround a regalloc bug.
-//
-// More information can be found in PR39553.
-def DREGS_WITHOUT_Z_WORKAROUND : RegisterClass<"AVR", [i16], 8,
- (
- // Return value and arguments.
- add R25R24, R19R18, R21R20, R23R22,
- // Scratch registers.
- R27R26,
- // Callee saved registers.
- R29R28, R17R16, R15R14, R13R12, R11R10,
- R9R8, R7R6, R5R4, R3R2, R1R0
- )>;
-
// 16-bit register class for immediate instructions.
def DLDREGS : RegisterClass<"AVR", [i16], 8,
( Just a guess (which may not be valid if the register allocator and coalsecer are clever enough): |
Yes, applying that does seem to improve the situation. I'll try again with libcore. |
I looked at it a bit more. Trying to disable the coalescence for register Z specifically would need to change the interface of (*) It may be much more complicated depending on how coalesced virtual register is handled when it is assigned a physical register. |
I looked at it even more and it is good to apply this on top of my patch: diff --git a/lib/Target/AVR/AVRRegisterInfo.cpp b/lib/Target/AVR/AVRRegisterInfo.cpp
index f31bd9970e9..ff15356030e 100644
--- a/lib/Target/AVR/AVRRegisterInfo.cpp
+++ b/lib/Target/AVR/AVRRegisterInfo.cpp
@@ -276,11 +276,11 @@ void AVRRegisterInfo::splitReg(unsigned Reg,
bool AVRRegisterInfo::shouldCoalesce(MachineInstr * /*MI*/,
const TargetRegisterClass * /*SrcRC*/,
unsigned /*SubReg*/,
- const TargetRegisterClass * /*DstRC*/,
+ const TargetRegisterClass * DstRC,
unsigned /*DstSubReg*/,
- const TargetRegisterClass *NewRC,
+ const TargetRegisterClass * /*NewRC*/,
LiveIntervals & /*LIS*/) const {
- return NewRC != &AVR::PTRDISPREGSRegClass;
+ return DstRC != &AVR::PTRDISPREGSRegClass;
}
} // end of namespace llvm Some coalescing on PTRDISPREGS could have still happened since NewRC can be nullptr when DstReg is physical. I did not notice that before. Any coalescing on PTRDISPREGS is disabled after this fix to my patch .... that is if the comments in the source code are correct. |
…leGen This regression was introduced in r351544, when the expansion for the LDWRdPtr instruction was changed, but the TableGen definition of the instruction was not. The expansion must now use the 'PTRDISPREGS' register class. Without this, any time the 'Z' register is selected for LDWRdPtr, invalid machine code (or an assertion error) would be generated. This patch modifies the LDWRdPtr instruction so that the Z register can never be selected to fill this pseudo. This bug was identified by Carl Peto in avr-rust/rust-legacy-fork#128.
Now that dylanmckay/llvm@9083457 has been merged, is that an upstream fix to the issue here? or does more work need to be done? |
Fixing register class from PTRREGS to PTRDISPREGS is a step in a good direction but it is not enough. It only replaces this error with error "ran out of registers". You need to disable coalescing on Z register completely. You may achieve that to a significant degree by applying all the patches I posted here. Unfortunately I do not know whether it is enough because I did not achieve any more progress. The primary reason for no progress on my side is because I'm blocked by unresolved dependency on compiler_builtins. So I cannot compile blink because of compiler_builtins. And I cannot compile compiler_builtins because it was missing dependency on std ... if I recall correctly. When I saw it I have given up because I expect std to depend on core which would make a nice dependency cycle :) If somebody can point me to some information how to bootstrap compile compiler_builtins for AVR then let me know and I may look at this again. |
@hercek, if this is the same issue I encountered, just adding a [dependencies.core]
stage = 0
[dependencies.compiler_builtins]
stage = 1
version = "= 0.1.3" should be enough to get |
Upstream fix accepted. |
If we would allow register coalescing on PTRDISPREGS class then register allocator can lock Z register to some virtual register. Larger instructions requiring a memory acces then fail during the register allocation phase since there is no available register to hold a pointer if Y register was already taken for a stack frame. This patch prevents it by keeping Z register spillable. It does it by not allowing coalescer to lock it. Original discussion on avr-rust/rust-legacy-fork#128. llvm-svn: 362298
I've also landed a commit upstream that disables register coalescing to the Y and Z registers. |
If we would allow register coalescing on PTRDISPREGS class then register allocator can lock Z register to some virtual register. Larger instructions requiring a memory acces then fail during the register allocation phase since there is no available register to hold a pointer if Y register was already taken for a stack frame. This patch prevents it by keeping Z register spillable. It does it by not allowing coalescer to lock it. Original discussion on avr-rust/rust-legacy-fork#128. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@362298 91177308-0d34-0410-b5e6-96231b3b80d8
If we would allow register coalescing on PTRDISPREGS class then register allocator can lock Z register to some virtual register. Larger instructions requiring a memory acces then fail during the register allocation phase since there is no available register to hold a pointer if Y register was already taken for a stack frame. This patch prevents it by keeping Z register spillable. It does it by not allowing coalescer to lock it. Original discussion on avr-rust/rust-legacy-fork#128. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@362298 91177308-0d34-0410-b5e6-96231b3b80d8
Hi! Is there any changes that have been made and committed? Could we get the blink program to work now? |
If we would allow register coalescing on PTRDISPREGS class then register allocator can lock Z register to some virtual register. Larger instructions requiring a memory acces then fail during the register allocation phase since there is no available register to hold a pointer if Y register was already taken for a stack frame. This patch prevents it by keeping Z register spillable. It does it by not allowing coalescer to lock it. Original discussion on avr-rust/rust-legacy-fork#128. llvm-svn: 362298
If we would allow register coalescing on PTRDISPREGS class then register allocator can lock Z register to some virtual register. Larger instructions requiring a memory acces then fail during the register allocation phase since there is no available register to hold a pointer if Y register was already taken for a stack frame. This patch prevents it by keeping Z register spillable. It does it by not allowing coalescer to lock it. Original discussion on avr-rust/rust-legacy-fork#128. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@362298 91177308-0d34-0410-b5e6-96231b3b80d8
If we would allow register coalescing on PTRDISPREGS class then register allocator can lock Z register to some virtual register. Larger instructions requiring a memory acces then fail during the register allocation phase since there is no available register to hold a pointer if Y register was already taken for a stack frame. This patch prevents it by keeping Z register spillable. It does it by not allowing coalescer to lock it. Original discussion on avr-rust/rust-legacy-fork#128.
Tbh I never really properly understood all this but it sounds good. Lol!
Sent from my BlackBerry® wireless device
…-----Original Message-----
From: Dylan McKay <[email protected]>
Date: Sat, 01 Jun 2019 05:37:31
To: avr-rust/rust<[email protected]>
Reply-To: avr-rust/rust <[email protected]>
Cc: Carl Peto<[email protected]>; Comment<[email protected]>
Subject: Re: [avr-rust/rust] Expected either Y or Z register (#128)
I've also landed a commit upstream that disables register coalescing to the Y and Z registers.
llvm/llvm-project@45eb4c7
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#128 (comment)
|
Nice :)
Sent from my BlackBerry® wireless device
…-----Original Message-----
From: Dylan McKay <[email protected]>
Date: Sat, 01 Jun 2019 01:34:59
To: avr-rust/rust<[email protected]>
Reply-To: avr-rust/rust <[email protected]>
Cc: Carl Peto<[email protected]>; Comment<[email protected]>
Subject: Re: [avr-rust/rust] Expected either Y or Z register (#128)
[Upstream fix](https://reviews.llvm.org/D60365) accepted.
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#128 (comment)
|
The invalid assembly generated error seems to be back when compiling blink.
The text was updated successfully, but these errors were encountered: