llvm dev - Sep 2017 - [MachineCopyPropagation] Issue with register forwarding/allocation/verifier in out-of-tree target

Hi all,

Mikael reported a machine verification failure in his out-of-tree target 
with the MachineCopyPropagation changes to forward registers (which is 
currently reverted).  The verification in question is:

*** Bad machine code: Multiple connected components in live interval ***
- function:    utils_la_suite_matmul_ref
- interval:    %vreg77 
[192r,208B:0)[208B,260r:1)[312r,364r:2)[380r,464B:3)  0 at 192r 1 at 208B-phi 
2 at 312r 3 at 380r
0: valnos 0 1 3
1: valnos 2

In this particular case, I believe that it is the greedy allocator that 
is creating the multiple components in the %vreg77 live interval.  If 
you look at the attached debug dump file, just after the greedy 
allocator runs, the segment of %vreg77 from the def at 312B to the use 
at 380B seems to be separable from the other segments.  The reason the 
above verification failure is not hit at that point seems to be related 
to the FIXME in the following snippet from 
ConnectedVNInfoEqClasses::Classify():

       // Normal value defined by an instruction. Check for two-addr redef.
       // FIXME: This could be coincidental. Should we really check for 
a tied
       // operand constraint?
       // Note that VNI->def may be a use slot for an early clobber def.
       if (const VNInfo *UVNI = LR.getVNInfoBefore(VNI->def))
         EqClass.join(VNI->id, UVNI->id);

Just after the greedy allocator runs, the instruction at 380B also 
defines %vreg77, so the verification check treats it as a two-addr 
redefinition (even though it is not) and allows it. 
MachineCopyForwarding renames the use of %vreg77 at 380B so the segment 
in question no longer ends at an instruction that is also a def, so the 
verification check fires.

It appears to me that this check is too loose, and if so that means 
there is something going wrong either in the allocator itself or in its 
interaction with this particular target in this case.

-- 
Geoff Berry
Employee of Qualcomm Datacenter Technologies, Inc.
  Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the Code 
Aurora Forum, a Linux Foundation Collaborative Project.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: foo.log.gz
Type: application/x-gzip
Size: 34687 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170926/97dd81ef/attachment.bin>

> On Sep 26, 2017, at 2:39 PM, Geoff Berry via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Hi all,
> 
> Mikael reported a machine verification failure in his out-of-tree target
with the MachineCopyPropagation changes to forward registers (which is currently
reverted).  The verification in question is:
> 
> *** Bad machine code: Multiple connected components in live interval ***
> - function:    utils_la_suite_matmul_ref
> - interval:    %vreg77 [192r,208B:0)[208B,260r:1)[312r,364r:2)[380r,464B:3)
0 at 192r 1 at 208B-phi 2 at 312r 3 at 380r
> 0: valnos 0 1 3
> 1: valnos 2
> 
> In this particular case, I believe that it is the greedy allocator that is
creating the multiple components in the %vreg77 live interval.  If you look at
the attached debug dump file, just after the greedy allocator runs, the segment
of %vreg77 from the def at 312B to the use at 380B seems to be separable from
the other segments.  The reason the above verification failure is not hit at
that point seems to be related to the FIXME in the following snippet from
ConnectedVNInfoEqClasses::Classify():
That dump seems to be well before greedy runs, isn't it?

At a first glance the odd thing there is that the operand of fladd_a32_a32_a32
is rewritten from vreg77 to vreg76, but the vreg77 operand of the BUNDLE is not.
Maybe you can find out why that is?

- Matthias

On 9/26/2017 6:11 PM, Matthias Braun wrote:
> 
>> On Sep 26, 2017, at 2:39 PM, Geoff Berry via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
>>
>> Hi all,
>>
>> Mikael reported a machine verification failure in his out-of-tree
target with the MachineCopyPropagation changes to forward registers (which is
currently reverted).  The verification in question is:
>>
>> *** Bad machine code: Multiple connected components in live interval
***
>> - function:    utils_la_suite_matmul_ref
>> - interval:    %vreg77
[192r,208B:0)[208B,260r:1)[312r,364r:2)[380r,464B:3)  0 at 192r 1 at 208B-phi 2
at 312r 3 at 380r
>> 0: valnos 0 1 3
>> 1: valnos 2
>>
>> In this particular case, I believe that it is the greedy allocator that
is creating the multiple components in the %vreg77 live interval.  If you look
at the attached debug dump file, just after the greedy allocator runs, the
segment of %vreg77 from the def at 312B to the use at 380B seems to be separable
from the other segments.  The reason the above verification failure is not hit
at that point seems to be related to the FIXME in the following snippet from
ConnectedVNInfoEqClasses::Classify():
> That dump seems to be well before greedy runs, isn't it?
I'm not sure what you mean.  The attached log contains -print-before-all 
-print-after-all and -debug output starting with the coalescer pass. 
The verification failure is right after the first pass of 
MachineCopyPropagation which runs after the greedy allocator.
> At a first glance the odd thing there is that the operand of
fladd_a32_a32_a32 is rewritten from vreg77 to vreg76, but the vreg77 operand of
the BUNDLE is not. Maybe you can find out why that is?
Sorry, I should have pointed this out before: because the loop over 
instructions in MachineCopyPropagation is only visiting the BUNDLE 
instructions themselves (i.e. it does not visit the instructions inside 
the BUNDLE) and we don't forward to implicit uses (which all of the 
BUNDLE operands are marked as), we won't currently forward a use to a 
bundled instruction.  I believe handling bundles more aggressively can 
be added as a follow-on enhancement unless we think not doing has an 
inherent problem.
> - Matthias
> 
-- 
Geoff Berry
Employee of Qualcomm Datacenter Technologies, Inc.
  Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the Code 
Aurora Forum, a Linux Foundation Collaborative Project.