llvm dev - Apr 2020 - 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: reduced.ll
Type: application/octet-stream
Size: 535 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200415/acf28f3d/attachment-0001.obj>

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

On Wed, Apr 15, 2020 at 8:34 PM Justin Hibbits via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> 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
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200415/d5da34a0/attachment.html>

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

On Wed, 15 Apr 2020 21:25:56 -0400
Nemanja Ivanovic <nemanja.i.ibm at gmail.com> wrote:
> 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
> 
> On Wed, Apr 15, 2020 at 8:34 PM Justin Hibbits via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> 
> > 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
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> >