New user self-registration is disabled due to spam. For an account please email bugs-admin@lists.llvm.org with your e-mail address and full name.

Bug 41121 - After r348496, "Unable to predicate BX killed renamable $r0"
Summary: After r348496, "Unable to predicate BX killed renamable $r0"
Status: RESOLVED FIXED
Alias: None
Product: new-bugs
Classification: Unclassified
Component: new bugs (show other bugs)
Version: trunk
Hardware: PC Windows NT
: P enhancement
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-03-18 02:57 PDT by Dimitry Andric
Modified: 2019-09-05 13:03 PDT (History)
10 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitry Andric 2019-03-18 02:57:21 PDT
As described in https://bugs.freebsd.org/236567, compiling the FreeBSD lang/spidermonkey170 or lang/spidermonkey38 ports for armv7 or armv6 errors out with (note the strange error formatting, this is verbatim):

Unable to predicate BX killed renamable $r0
!
UNREACHABLE executed at lib/CodeGen/IfConversion.cpp:2022!

Bisection shows this started occurring after https://reviews.llvm.org/rL348496 ("[GVN] Don't perform scalar PRE on GEPs").

Minimized test case:

// clang -cc1 -triple armv7-- -S -O2 Unified_cpp_js_src10-min.cpp
template <typename> struct c;
template <typename b> struct j : c<b> {
  j(int);
  b *aa() { return d; }
  b e() { return *d; }
  b *d;
};
typedef struct {
  struct {
    union {
      int f;
      int *ab;
    } g;
  } h;
} i;
struct n {
  int *k() { return ac.h.g.ab; }
  int l() { return ac.h.g.f; }
  i ac;
};
template <class m> struct p {
  int ad();
  int *k() {
    n o = *static_cast<m *>(this)->ae();
    return o.k();
  }
};
template <> struct c<n> : p<j<n>> {
  n *ae() { return static_cast<j<n> *>(this)->aa(); }
};
void fn1(bool *);
bool q;
int r;
void u() {
  void *a[]{&&s, &&t};
t:
  fn1(&q);
  goto *a[r];
s:;
}
int v(j<n> w, j<n> x, bool *y) {
  if (w.ad()) {
    *y = w.k() == x.k();
    return 0;
  }
  *y = w.e().l() == x.e().l();
  return 0;
}
void fn1(bool *w) { v(0, 0, w); }
Comment 1 Eli Friedman 2019-03-18 16:32:35 PDT
I spent some time tracing this.

It looks like there's a bug in IfConverter::CountDuplicatedInstructions in cases where a block has a tail which can't be analyzed.  In practice, this probably means an indirect goto or jumptable (or maybe some exception-handling stuff).  The problem is that Dups1/Dups2 return the wrong result: the logic decided earlier that the branches should be treated as opaque... but then we skip them when deciding the number of duplicate instructions.  This means we underestimate the number of identical instructions, and therefore try to predicate instructions which don't need to be predicated.

In many cases, this would be missed optimization, not a crash... but in the given testcase, the instruction we try to predicate can't be predicated, so it causes an assertion failure.  (ARM::BX probably should be predicable, actually, but I think it's possible to make the logic try to predicate an arbitrary instruction, and some instructions really can't be predicated.)

Not sure what the right fix looks like yet; the logic around analyzable vs. non-analyzable branches is sort of tricky.
Comment 2 Eli Friedman 2019-09-05 13:03:06 PDT
r371111