[PATCH] D51028: [BranchFolder] Drop kill/dead flags if they aren't present in all merged instructions

[Mikael Holmén via Phabricator via llvm-commits]

uabelho added a comment.



In D51028#1839423 <https://reviews.llvm.org/D51028#1839423>, @jonpa wrote:

> I am a little curious as to how these differences in flags arise. I can understand that a kill flag can be seen as an optimization and is optional. The same would go for a dead flag. But I am not sure about an undef flag... If it is present in one case but not in the other, the two instructions do not seem equivalent, or?
>
> So I suppose your patch makes sense to me, although I am not familiar enough to understand the undef case... Perhaps some comment on how these three cases can emerge and may be treated would be good?  (at least to me :-)


One full llc reproducer for my out-of-tree target showing that we need to clear the dead flag would look like this:

  @e = external global i16, align 1
  
  declare void @q(i16, i16, i16, i16, i16)
  
  define void @x() {
  bb.0:
    %tmp = alloca i16, align 1
    %tmp3 = alloca i16, align 1
    br i1 undef, label %bb.2, label %bb.1
  
  bb.1:                                             ; preds = %bb.0
    %0 = load volatile i16, i16* @e, align 1
    store i16 0, i16* %tmp3, align 1
    tail call void @q(i16 undef, i16 undef, i16 undef, i16 undef, i16 %0)
    unreachable
  
  bb.2:                                             ; preds = %bb.0
    %1 = load volatile i16, i16* @e, align 1
    store i16 0, i16* %tmp, align 1
    tail call void @q(i16 undef, i16 undef, i16 undef, i16 undef, i16 undef)
    unreachable
  }

Note the difference in the last argument to function q in bb.1 and bb2.

When we reach the BranchFolder the load and the setting up of the last
argument (which should be passed on the stack) has turned into the following:

In bb.1:

  $r0 = mv16Sym @e
  $a0h = mv_r16_rmod1_ar16 killed $r0
  push_any16 killed $a0h

and in bb.2:

  $r0 = mv16Sym @e
  dead $a0h = mv_r16_rmod1_ar16 killed $r0
  push_any16 undef $a0h

The BranchFolder wants to merge the above and without the clearing of the "dead"
flag in this patch we get:

  $r0 = mv16Sym @e
  dead $a0h = mv_r16_rmod1_ar16 killed $r0
  push_any16 $a0h

and then the verifier pukes.

My patch removes the "dead" above so we instead get

  $r0 = mv16Sym @e
  $a0h = mv_r16_rmod1_ar16 killed $r0
  push_any16 $a0h

which the verifer is happy with and I see nothing wrong with.

So in this case, an "undef" in the llc input could lead to a verifier error.


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

https://reviews.llvm.org/D51028