Need help figuring out a isNopCopy() assert

I'm trying to fix a bug in the PowerPC SPE backend that prevents a
bunch of FreeBSD ports from building, including gtk20. The attached
file, generated from the following C source, triggers the "Def ==
PreviousDef" assertion in isNopCopy():

typedef float a;
typedef struct {
  a b, c;
} complex;
d(complex *e, complex *h) {
  double f = h->c, g = h->b;
  i(g);
  e->c = g * j(f);
}

This was a reduction from texlive's c_cos.c.

The applicable assertion failure is:

Assertion failed: (Def == PreviousDef), function isNopCopy, file
/home/chmeee/llvm_git/llvm/llvm/lib/CodeGen/MachineCopyPropagation.cpp,
line 338. PLEASE submit a bug report to https://bugs.llvm.org/ and
include the crash backtrace. Stack dump:
0. Program arguments: /ralga/scratch/chmeee/builds/llvm/bin/llc
-mtriple powerpcspe reduced.ll
1. Running pass 'Function Pass Manager' on module 'reduced.ll'.
2. Running pass 'Machine Copy Propagation Pass' on function '@d'

I dumped the debug during this, and found the following interesting
bits:

0B bb.0.entry:
          liveins: $r4
16B %1:gprc_and_gprc_nor0 = COPY $r4
32B %2:gprc = SPELWZ 0, undef %3:gprc_and_gprc_nor0 :: (load 4
from `float* undef`) 40B %5:gprc = SPELWZ 0, killed
%1:gprc_and_gprc_nor0 :: (load 4 from %ir.b1) 48B %4:sperc = COPY
killed %2:gprc 80B %6:sperc = COPY killed %5:gprc

...
MCP: Copy is a deletion candidate: renamable $s29 = COPY killed
renamable $r4
MCP: Copy is used - not dead: renamable $s29 = COPY
killed renamable $r4
MCP: Copy is used - not dead: renamable $s29 = COPY killed renamable
$r4

$s29 is a SPE register (64-bits wide), while $r4 is a GPR (32-bits
wide), and given what I've found googling this assert, the assert
indicates two different register classes, which is obvious from this.
The question is *where* and *why* do I have this?

At first I thought to look through PPCFastISel.cpp, since there's some
code in there that does fast selection with assumptions of the
traditional PowerPC FPU. However, that didn't pan out. Now I'm
scratching my head, and looking for help on where to try next, as every
place I've found "TargetOpcode::COPY" and "PPC::COPY" appear to be
covered already, correctly.

Thanks,
Justin

reduced.ll (535 Bytes)

Your pattern says to try to do that impossible copy. When you have a COPY_TO_REGCLASS with different size registers, you end up getting some weirdness. You probably want something along these lines:
diff --git a/llvm/lib/Target/PowerPC/PPCInstrSPE.td b/llvm/lib/Target/PowerPC/PPCInstrSPE.td
index 935c304…ec108b3 100644
— a/llvm/lib/Target/PowerPC/PPCInstrSPE.td
+++ b/llvm/lib/Target/PowerPC/PPCInstrSPE.td
@@ -821,9 +821,9 @@ def SPESTWX : XForm_8<31, 151, (outs), (ins spe4rc:$rS, memrr:$dst),

let Predicates = [HasSPE] in {
def : Pat<(f64 (extloadf32 iaddr:$src)),

  • (COPY_TO_REGCLASS (SPELWZ iaddr:$src), SPERC)>;
  • (SUBREG_TO_REG (i64 1), (SPELWZ iaddr:$src), sub_32)>;
    def : Pat<(f64 (extloadf32 xaddr:$src)),
  • (COPY_TO_REGCLASS (SPELWZX xaddr:$src), SPERC)>;
  • (SUBREG_TO_REG (i64 1), (SPELWZX xaddr:$src), sub_32)>;

def : Pat<(f64 (fpextend f32:$src)),

(COPY_TO_REGCLASS $src, SPERC)>;

That allows the test case to compile correctly but I haven’t looked at the details of the produced code. You should have a look at all of your uses of COPY_TO_REGCLASS to see whether that is what you want or if it is a different output pattern you’re after.

N

Hi Nemanja,

Thanks for that quick reply. Your fix does indeed fix that assert.
But, as you pointed out on IRC, the pattern was wrong to begin with,
and the COPY_TO_REGCLASS needs to instead be a EFDCFS, so it becomes:
<snipped for brevity>

- (COPY_TO_REGCLASS (SPELWZ iaddr:$src), SPERC)>;
+ (EFDCFS (SPELWZ iaddr:$src))>;
def : Pat<(f64 (extloadf32 xaddr:$src)),
- (COPY_TO_REGCLASS (SPELWZX xaddr:$src), SPERC)>;
+ (EFDCFS (SPELWZX xaddr:$src))>;

I got led astray by misunderstanding the FPU patterns. However, also
mentioned, since this is manually expanding it in a pattern, we should
be able to just let LLVM do the work instead, and remove these
patterns, marking extended loads as illegal.

Thanks for the help.

- Justin