[AVR] Fix a bug in selection of ANY_EXTEND

[benshi001]

This is a walk around solution of #132203

[Patryk27]

I can't reproduce the original crash - this compiles successfully on my LLVM (fbaf3b8, i.e. main from a couple of minutes ago).

[benshi001]

I can't reproduce the original crash - this compiles successfully on my LLVM (fbaf3b8, i.e. main from a couple of minutes ago).

Did you do -DLLVM_ENABLE_EXPENSIVE_CHECKS=On before building llvm from source? Maybe you can try this -DLLVM_ENABLE_EXPENSIVE_CHECKS=On when doing cmake ?

[beakthoven]

I can't reproduce the original crash - this compiles successfully on my LLVM (fbaf3b8, i.e. main from a couple of minutes ago).

Did you do -DLLVM_ENABLE_EXPENSIVE_CHECKS=On before building llvm from source? Maybe you can try this -DLLVM_ENABLE_EXPENSIVE_CHECKS=On when doing cmake ?

Can confirm, with expensive checks enabled the crash occurs, otherwise it doesn't.

[Patryk27]

Ah, that's a nice catch, I have that turned off - let me try again.

[Patryk27]

Minimized:

define i16 @main(i8 %a) {
  %1 = zext i8 %a to i16
  %2 = sub i16 %1, 256
  %3 = and i16 %2, 1

  ret i16 %3
}

I'll play a bit and see if I can dig to the underlying issue.

[Patryk27]

For others - the issue is that we end up generating a 16-bit instruction (subtraction, addition, ...) on a partially initialized register-pair.

Following my simplified example, after avr-isel we get:

bb.0 (%ir-block.0):
  liveins: $r24
  %0:gpr8 = COPY $r24
  %2:dregs = IMPLICIT_DEF
  %1:dldregs = INSERT_SUBREG %2:dregs(tied-def 0), %0:gpr8, %subreg.sub_lo
  %3:dldregs = SUBIWRdK %1:dldregs(tied-def 0), 256, implicit-def dead $sreg
  %4:dldregs = ANDIWRdK %3:dldregs(tied-def 0), 1, implicit-def dead $sreg
  $r25r24 = COPY %4:dldregs
  RET implicit $r25r24, implicit $r1

... which later gets expanded into:

0B	bb.0 (%ir-block.0):
	  liveins: $r24
16B	  undef %4.sub_lo:dldregs = COPY $r24
64B	  %4:dldregs = SUBIWRdK %4:dldregs(tied-def 0), 256, implicit-def dead $sreg
96B	  %4:dldregs = ANDIWRdK %4:dldregs(tied-def 0), 1, implicit-def dead $sreg
112B	  $r25r24 = COPY %4:dldregs
128B	  RET implicit $r25r24, implicit $r1

... and into (via virtregrewriter - note the lost information about partially initialized $r25r24):

0B	bb.0 (%ir-block.0):
	  liveins: $r24
64B	  $r25r24 = SUBIWRdK $r25r24(tied-def 0), 256, implicit-def dead $sreg
96B	  $r25r24 = ANDIWRdK killed $r25r24(tied-def 0), 1, implicit-def dead $sreg
128B	  RET implicit $r25r24, implicit $r1

... into:

bb.0 (%ir-block.0):
  liveins: $r24
  $r24 = SUBIRdK $r24(tied-def 0), 0, implicit-def $sreg
  $r25 = SBCIRdK $r25(tied-def 0), 1, implicit-def dead $sreg, implicit killed $sreg
  $r24 = ANDIRdK killed $r24(tied-def 0), 1, implicit-def dead $sreg
  $r25 = ANDIRdK killed undef $r25(tied-def 0), 0, implicit-def dead $sreg
  RET implicit $r25r24, implicit $r1

Note that the second ANDIRdK correctly says undef $r25, but that's just a happy accident thanks to

llvm-project/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp

Line 281 in f3e8e80

MIBHI->getOperand(1).setIsUndef(true);

tl;dr it looks like somewhere in-between avr-isel and virtregrewriter we lose the information about "undef-ness" of half of the assigned register-pair - I'm not yet sure what's the correct solution (maybe explicitly inspecting liveins?), but the proposed hotfix seems to be correct (though overzealous, as noted in the commented code).

[Patryk27]

I've got a hunch this is how AArch64 solves this problem:

llvm-project/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

Line 6195 in 0d3ba08

if (FillRC) {

[benshi001]

I've got a hunch this is how AArch64 solves this problem:

llvm-project/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

Line 6195 in 0d3ba08

if (FillRC) {

Yes. I will make a formal solution later, after I fix issue #127651 .