Skip to content
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

[SystemZ] Bad machine code: Illegal virtual register for instruction #121232

Open
JonPsson1 opened this issue Dec 27, 2024 · 0 comments
Open

[SystemZ] Bad machine code: Illegal virtual register for instruction #121232

JonPsson1 opened this issue Dec 27, 2024 · 0 comments

Comments

@JonPsson1
Copy link
Contributor

llc -mtriple=s390x-linux-gnu -mcpu=z14 -O3 -enable-deferred-spilling -stress-regalloc=8 -verify-machineinstrs ./tc_memfold_vr128.mir -start-before=greedy -o /dev/null

*** Bad machine code: Illegal virtual register for instruction ***
- function:    main
- basic block: %bb.4  (0x93b8a08) [320B;416B)
- instruction: 352B	%22:fp32bit = nofpexcept MAEB_MemFoldPseudo %19:fp32bit, %60.subreg_h32:vr128bit, %stack.0, 0, $noreg, implicit $fpc :: (load (s32) from %stack.0)
- operand 2:   %60.subreg_h32:vr128bit
Expected a VF128Bit register, but got a VR128Bit register

tc_memfold_vr128.tar.gz

// In input:

%11:vr128bit = VGMF 2, 7
%22:fp32bit = nofpexcept WFMASB %55:fp32bit, %52.subreg_h32:vr128bit, %19:fp32bit, implicit $fpc

// %55 is spilled and a reg/mem is produced with %52 constrained to VF128Bit in SystemZInstrInfo::foldMemoryOperandImpl():

selectOrSplit FP32Bit:%55
   folded:   464r  %22:fp32bit = nofpexcept MAEB_MemFoldPseudo %19:fp32bit, %52.subreg_h32:vf128bit, %stack.0, 0, $noreg

// Then %52 is also spilled, but it is rematerialized with a new vreg of the original RC:

Inline spilling VF128Bit:%52 [200r,1376B:0) 0@200r  weight:1.375309e+03
From original %11
        remat:  456r    %60:vr128bit = VGMF 2, 7
                464e    %22:fp32bit = nofpexcept MAEB_MemFoldPseudo %19:fp32bit, killed %60.subreg_h32:vr128bit, %stack.0, 0, $noreg

// InlineSpiller create a new VirtReg with same RC as Original here:

InlineSpiller::reMaterializeFor() {
...
  // Allocate a new register for the remat.
  Register NewVReg = Edit->createFrom(Original);
...
}

As this only happens with -enable-deferred-spilling, I am not quite sure if this needs fixing, even though it in theory it probably should be.

TargetInstrInfo::reMaterialize() is virtual, and it would make sense to override this and in case of a MemFoldPseudo make sure all VR128 regs are constrained to VF128.

It wouldn't hurt if InlinSpiller / LiveRangeEdit (or even BaseT::reMaterialize() ) could constrain the vreg, but it does not check the MCDescriptor. As it is constrained by the target in the first place, maybe the target should keep that responsibility in reMaterialize().

@uweigand @dominik-steenken

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant