This is an archive of the discontinued LLVM Phabricator instance.

[UnreachableBlockElim] Check return value of constrainRegClass().
ClosedPublic

Authored by uabelho on May 9 2017, 6:05 AM.

Details

Summary

MachineRegisterInfo::constrainRegClass() can fail if two register classes
don't have a common subclass or if the register class doesn't contain
enough registers. Check the return value before trying to remove Phi nodes,
and if we can't constrain, we output a COPY instead of simply replacing
registers.

Diff Detail

Event Timeline

uabelho created this revision.May 9 2017, 6:05 AM

Not sure who is code owner of IfConversion.cpp so I added the last three persons who changed
anything there as reviwers.

I found this problem on my out-of-tree target. I've tried to make a reproducer also on Hexagon
but I failed.

My out-of-tree test case looks like:

#RUN: llc -O0 -mtriple phoenix -mcpu=phoenixV -o - %s -run-pass=unreachable-mbb-elimination | FileCheck %s

  • | define void @foo() { bb0: %_tmp0 = load i16*, i16** undef, align 1 br label %bb1

    bb1: ; preds = %bb0 %_tmp4 = phi i16* [ %_tmp0, %bb0 ] %_tmp5 = icmp eq i16* %_tmp4, undef br label %bb2

    bb2: ; preds = %bb1 ret void }

...

name: foo
alignment: 0
exposesReturnsTwice: false
noVRegs: false
legalized: false
regBankSelected: false
selected: false
tracksRegLiveness: true
registers:

  • { id: 0, class: rn }
  • { id: 1, class: anh_0_7 }
  • { id: 2, class: rn }
  • { id: 3, class: anh_0_7 }

frameInfo:

isFrameAddressTaken: false
isReturnAddressTaken: false
hasStackMap:     false
hasPatchPoint:   false
stackSize:       0
offsetAdjustment: 0
maxAlignment:    0
adjustsStack:    false
hasCalls:        true
maxCallFrameSize: 0
hasOpaqueSPAdjustment: false
hasVAStart:      false
hasMustTailInVarArgFunc: false

body: |

bb.0:
  successors: %bb.1

  %2 = IMPLICIT_DEF
  %0 = mv_r16_rmod1_ar16 killed %2, 0, _, 0, implicit %dc0, implicit %dc1, implicit %ac0, implicit %ac1
  brr_uncond %bb.1

bb.1:
  successors: %bb.2

  %1 = PHI %0, %bb.0
  %3 = IMPLICIT_DEF
  cmp_a16_a16 %1, killed %3, 0, _, 0, implicit-def %ccreg
  brr_uncond %bb.2

bb.2:
  rets 0, _, 0, implicit-def dead %sp, implicit %sp, implicit %cuc

...

  1. The important part here is that we do not use %0 directly in the cmp since %0
  2. has regclass rn and the cmp only accepts anh_0_7. So we need a copy from %0
  3. to %1, and %1 has regclass anh_0_7

#CHECK: registers:
#CHECK: - { id: 0, class: rn }
#CHECK: - { id: 1, class: anh_0_7 }

#CHECK: bb.0:
#CHECK: successors: %bb.1
#CHECK: %0 = mv_r16_rmod1_ar16 killed %2

#CHECK: bb.1:
#CHECK: %1 = COPY %0
#CHECK: cmp_a16_a16 %1, killed %3

kparzysz accepted this revision.May 9 2017, 8:04 AM

LGTM with an update to the comment.

lib/CodeGen/UnreachableBlockElim.cpp
213–214

You should update the comment as well.

This revision is now accepted and ready to land.May 9 2017, 8:04 AM
uabelho updated this revision to Diff 98400.May 9 2017, 10:13 PM

Updated a comment according to review comments.

uabelho marked an inline comment as done.May 9 2017, 10:14 PM
This revision was automatically updated to reflect the committed changes.