[PATCH] D156345: RegisterCoalescer: Add implicit-def of super register when coalescing SUBREG_TO_REG

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/RegisterCoalescer.cpp:2163
+  if (CP.getDstIdx()) {
+    assert(!IsSubregToReg && "can this happen?");
+    updateRegDefsUses(CP.getDstReg(), CP.getDstReg(), CP.getDstIdx(), false);
----------------
arsenm wrote:
> arsenm wrote:
> > arsenm wrote:
> > > qcolombet wrote:
> > > > qcolombet wrote:
> > > > > Maybe it can happen if you have something like:
> > > > > ```
> > > > > %a = SUBREG_TO_REG ...
> > > > > %b = IMPLICIT_DEF
> > > > > %c = INSERT_SUBREG %b, %a, sub
> > > > > ```
> > > > > =>
> > > > > ```
> > > > > %a = SUBREG_TO_REG ...
> > > > > %c.sub = COPY %a
> > > > > ```
> > > > > =>
> > > > > ```
> > > > > %c.sub = SUBREG_TO_REG ...
> > > > > ```
> > > > Ditto on the can this happen.
> > > > Did you give a try to the example I gave?
> > > If I force subregister liveness on ninja check does find the failures.
> > > 
> > > Also the baseline ninja check with verify-coalescing forced on isn't clean :(
> > This does hit the assert / verifier error, so I'll push it somewhere
> > 
> > ```
> > 
> > # RUN: llc -mtriple=x86_64-- -run-pass=register-coalescer -enable-subreg-liveness -o - %s
> > ---
> > name:            test
> > tracksRegLiveness: true
> > body:             |
> >   bb.0:
> >     liveins: $eax
> >     %init_eax:gr32 = COPY $eax
> >     %a:gr64 = SUBREG_TO_REG 0, %init_eax, %subreg.sub_32bit
> >     %b:gr32 = IMPLICIT_DEF
> >     %c:gr64 = INSERT_SUBREG %a, %b, %subreg.sub_32bit
> >     JCC_1 %bb.2, 4, implicit undef $eflags
> > 
> >   bb.1:
> >     %imm0:gr32 = MOV32r0 implicit-def dead $eflags
> >     %a = SUBREG_TO_REG 0, %imm0, %subreg.sub_32bit
> >     %c.sub_32bit = COPY %a
> > 
> >   bb.2:
> >     %c.sub_32bit = SUBREG_TO_REG %a, %b, %subreg.sub_32bit
> >     RET 0, implicit %c
> > 
> > ...
> > 
> > ```
> For some reason this failure only manifests further up the stack of patches 
Oh, it's because this required hacking in enabling subreg liveness for x86. The flag apparently doesn't override the lack of implementation of the target hook 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156345/new/

https://reviews.llvm.org/D156345