llvm dev - Aug 2015 - [LLVMdev] TableGen Register Class not matching for MI in 3.6

> On Aug 22, 2015, at 9:10 AM, Ryan Taylor <ryta1203 at gmail.com>
wrote:
> 
> One last question regarding this please.
> 
> Why aren't we simply changing the register class in AddRegisterOperand
instead of building a new COPY? I admit I haven't thought this out but for
my test cases so far this works just fine and reduces the number of ASM mov
instructions that are being produced.
> 
> For example, instead of BuildMI(..., TII->get(TargetOpcode::COPY),
NewVReg).addReg(VReg), use something like MRI->setRegClass(VReg,
MRI->getRegClass(NewVReg)) ?
The problem is that the old register class and the new one may not intersect. I
do not know exactly what makes us create a new vreg w.r.t., but probably if the
class is not identical we create one. You can try to use contraintsRegClass to
get the intersection.

Q.
> 
> What is the reasoning behind adding the extra instruction instead of
changing the reg class?
> 
> On Wed, Aug 19, 2015 at 2:04 PM, Ryan Taylor <ryta1203 at gmail.com
<mailto:ryta1203 at gmail.com>> wrote:
> Yes, you're probably right about the ID. The odd part is that I have
other simpler instructions that use the same type of superset and it always, so
far, matches correctly (it doesn't just pick GPRRegs all the time).
> 
> Like I said, we can just 'fill in the gaps' with new MIs but that
sure seems like a brush off solution. The td files would be so much cleaner if
you could have a superset reg class that actually matched the correct reg usage
(which it sort of does in AddRegisterOperand when it adds the extra COPY.... not
sure why it does this instead of just checking the original MI and seeing if the
reg class needed is in the list and then just changing the vreg reg class for
the original MI, that seems like a better solution?) It's like it's
actually picking some reg class first and then trying to fix it's error by
adding MORE instructions instead of finding the right reg class the first time.
> 
> Thanks.
> 
> On Wed, Aug 19, 2015 at 1:32 PM, Quentin Colombet <qcolombet at
apple.com <mailto:qcolombet at apple.com>> wrote:
> 
>> On Aug 19, 2015, at 9:42 AM, Ryan Taylor <ryta1203 at gmail.com
<mailto:ryta1203 at gmail.com>> wrote:
>> 
>> It seems the problem arises from using multiple reg classes for one MI
in the td file, I guess.
> 
> Probably, that does not sound something used widely :).
> 
> 
>> I'm not sure it takes first available, if I swap the reg classes in
the list it does not change and if I replace the GPR reg class with something
different than it picks the base reg class fine, potentially it is using the reg
class with most available? idk.
> 
> My guess is it would take the register class that come first in the
register class IDs (not the list on the instruction itself), but I am just
guessing.
> 
>> 
>> I just need to create MIs for every possible case I guess.
>> Thanks for the help! :)
>> 
>> On Wed, Aug 19, 2015 at 12:04 PM, Quentin Colombet <qcolombet at
apple.com <mailto:qcolombet at apple.com>> wrote:
>> Hi Ryan,
>> 
>>> On Aug 19, 2015, at 6:35 AM, Ryan Taylor via llvm-dev <llvm-dev
at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>>> 
>>> Essentially it doesn't appear that the reg class assignment is
based on uses and is instead inserting an extra COPY for this. Is this accurate?
If so, why?
>> 
>> We match the instructions bottom-up and I believe that copy are
automatically inserted when the register classes do not match.
>> That seems strange to me that the isel logs are exactly the same but
still you are seeing a different result of instruction selection.
>> 
>>> 
>>> In this above example, I'm getting an extra "mov %r0,
$b1" (this is an MI::COPY) even though "mov @a, %b1" (this is an
MI::MOV) is entirely acceptable since both GPRRegs and BaseRegs are in the reg
class list..
>>> 
>>> If the heuristic is simply picking the first available reg class or
the reg class with the most available, for example, instead of picking the reg
class based on uses, I will have to change this heuristic to reduce code bloat,
or we'll have to add backend passes to reduce the code bloat.
>> 
>> I think the current approach is rather simple and we just match the
type for the first available reg class (assuming your selection dag patterns are
not choosing something different). There are not per say passes to reduce this
"code bloat” since we never encounter this problem in practice. The
peephole optimizer has some logic to avoid this move and CodeGenPrepare also
does a few transformation to avoid some of that.
>> 
>> Anyhow, I’d say debug the isel process (probably the InstrEmitter part)
to see where the problem arise.
>> 
>> Cheers,
>> -Quentin
>> 
>>> 
>>> Thanks.
>>> 
>>> On Mon, Aug 17, 2015 at 7:00 PM, Ryan Taylor <ryta1203 at
gmail.com <mailto:ryta1203 at gmail.com>> wrote:
>>> 
>>> ---------- Forwarded message ----------
>>> From: Ryan Taylor <ryta1203 at gmail.com <mailto:ryta1203 at
gmail.com>>
>>> Date: Mon, Aug 17, 2015 at 6:59 PM
>>> Subject: Re: [LLVMdev] TableGen Register Class not matching for MI
in 3.6
>>> To: Quentin Colombet <qcolombet at apple.com
<mailto:qcolombet at apple.com>>
>>> Cc: "llvmdev at cs.uiuc.edu <mailto:llvmdev at
cs.uiuc.edu>" <llvmdev at cs.uiuc.edu <mailto:llvmdev at
cs.uiuc.edu>>
>>> 
>>> 
>>> The isel logs are exactly the same, nothing changed, it's
matching the same instructions just the reg classes are different.
>>> 
>>> Where in the SelectionDAG is the code that adds an extra MI COPY?
I'll have to set a breakpoint and debug backwards from there to see
what's going on. It's only happening with the GPRRegs class, for example
if I use another reg class instead along with the base regs than it matches the
base reg just fine, it's like GPR is overriding any other reg class
matching.
>>> 
>>> Thanks.
>>> 
>>> On Fri, Jul 31, 2015 at 1:21 PM, Quentin Colombet <qcolombet at
apple.com <mailto:qcolombet at apple.com>> wrote:
>>> 
>>>> On Jul 31, 2015, at 10:14 AM, Ryan Taylor <ryta1203 at
gmail.com <mailto:ryta1203 at gmail.com>> wrote:
>>>> 
>>>> Quentin,
>>>> 
>>>>  It's in the instruction selection, sorry I forgot to
mention that. The Vreg class is GPR and an extra COPY is generated to copy from
the GPR to the Base Reg, even though my 'mov' instruction has Base in
the Register class list.
>>> 
>>> Then, I suggest you use -debug-only=isel to check what changed in
your selection instruction process. I do not see off-hand what could have
caused.
>>> 
>>> Sorry for not being more helpful here.
>>> 
>>> Q.
>>> 
>>>> 
>>>> 
>>>> On Fri, Jul 31, 2015 at 12:50 PM, Quentin Colombet
<qcolombet at apple.com <mailto:qcolombet at apple.com>> wrote:
>>>> Hi Ryan,
>>>> 
>>>> Could you check where those moves come from?
>>>> 
>>>> In particular, is this the product of the instruction selection
process?
>>>> 
>>>> You use -print-machineinstrs to see when it is inserted.
>>>> 
>>>> Thanks,
>>>> -Quentin
>>>> 
>>>> > On Jul 30, 2015, at 2:02 PM, Ryan Taylor <ryta1203 at
gmail.com <mailto:ryta1203 at gmail.com>> wrote:
>>>> >
>>>> > In LLVM 3.6,
>>>> >
>>>> >  We have an instruction that uses a register class that is
defined of several different reg classes. In 3.4 this works fine but in 3.6 this
is broken.
>>>> >
>>>> > For example, I have a mov instruction. mov can be executed
between different register types (ie gpr, index, base, etc..)
>>>> >
>>>> > In 3.4, we would get something like this:
>>>> >
>>>> > mov @a, %b1 // moving this immediate to a base register,
which is what we want
>>>> >
>>>> > In 3.6, we now get this:
>>>> >
>>>> > mov @a, %r0  // r0 = gpr
>>>> > mov %r0, %b1 // b1 = base reg
>>>> >
>>>> > The register class looks like this:
>>>> >
>>>> > def ARegs : RegisterClass<"us", [i16], i16,
(add GPRRegs, IndexRegs, BaseRegs)>;
>>>> >
>>>> > I have absolutely no idea why this is not matching any
longer?
>>>> >
>>>> > The fix here is to define an MI with explicit single
register class (ie it only allows PTRRegs as the destination).
>>>> >
>>>> > This must be an issue with something else and not the
tablegen but if that was the case I'm not sure. Anyway help would be great,
what should I be looking at here?
>>>> >
>>>> > Thanks.
>>>> > _______________________________________________
>>>> > LLVM Developers mailing list
>>>> > LLVMdev at cs.uiuc.edu <mailto:LLVMdev at
cs.uiuc.edu>         http://llvm.cs.uiuc.edu
<https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.cs.uiuc.edu_&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=7VQTHNyB_IcF4I86741RzVduPmdgCRL_mHUfuUmnL0Y&s=fK8KXlXNFjX9EyldRbiY4GwaxZBFjvTYEXwRJuFLJvg&e=>
>>>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
<https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.cs.uiuc.edu_mailman_listinfo_llvmdev&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=7VQTHNyB_IcF4I86741RzVduPmdgCRL_mHUfuUmnL0Y&s=TAPJBdWJfY-53g2FEN-kOKTo2nDJxcpEGNpqpgcrN9U&e=>
>>>> 
>>>> 
>>> 
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>
https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Ddev&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=7VQTHNyB_IcF4I86741RzVduPmdgCRL_mHUfuUmnL0Y&s=N0TOaZZSrDVRpbh_uMG5DV5ZZ2_KVMcBWtK9vGIaByY&e=
<https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Ddev&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=7VQTHNyB_IcF4I86741RzVduPmdgCRL_mHUfuUmnL0Y&s=N0TOaZZSrDVRpbh_uMG5DV5ZZ2_KVMcBWtK9vGIaByY&e=>
>> 
>> 
> 
> 
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150824/df8346d5/attachment.html>

I'm trying to do something like this:

// Dst = NewVReg's reg class
// *II = MCInstrDesc
// IIOpNum = II Operand Num

if (TRI->getCommonSubClass(DstRC,
TRI->getRegClass(II->OpInfo[IIOpNum].RegClass)) == DstRC)
     MRI->setRegClass(VReg, DstRC);
else
     BuildMI(... TargetOpcode::COPY...)

The condition is trying to reset the reg class if the DstRC reg class is
valid for the operand num of the machine instruction. If the NewVReg
register class is not valid for that operand of the machine instruction I
want to generate a COPY instruction (as it does now all the time). This
condition should check for intersection, right? It should find the common
subclass of the new register class and the valid reg class of the operand
for that machine instruction.

Unfortunately it looks like that condition is always being set to true (or
I'm getting a TON of false positives).

Thanks.

On Mon, Aug 24, 2015 at 2:09 PM, Quentin Colombet <qcolombet at apple.com>
wrote:
>
> On Aug 22, 2015, at 9:10 AM, Ryan Taylor <ryta1203 at gmail.com>
wrote:
>
> One last question regarding this please.
>
> Why aren't we simply changing the register class in AddRegisterOperand
> instead of building a new COPY? I admit I haven't thought this out but
for
> my test cases so far this works just fine and reduces the number of ASM mov
> instructions that are being produced.
>
> For example, instead of BuildMI(..., TII->get(TargetOpcode::COPY),
> NewVReg).addReg(VReg), use something like MRI->setRegClass(VReg,
> MRI->getRegClass(NewVReg)) ?
>
>
> The problem is that the old register class and the new one may not
> intersect. I do not know exactly what makes us create a new vreg w.r.t.,
> but probably if the class is not identical we create one. You can try to
> use contraintsRegClass to get the intersection.
>
> Q.
>
>
> What is the reasoning behind adding the extra instruction instead of
> changing the reg class?
>
> On Wed, Aug 19, 2015 at 2:04 PM, Ryan Taylor <ryta1203 at gmail.com>
wrote:
>
>> Yes, you're probably right about the ID. The odd part is that I
have
>> other simpler instructions that use the same type of superset and it
>> always, so far, matches correctly (it doesn't just pick GPRRegs all
the
>> time).
>>
>> Like I said, we can just 'fill in the gaps' with new MIs but
that sure
>> seems like a brush off solution. The td files would be so much cleaner
if
>> you could have a superset reg class that actually matched the correct
reg
>> usage (which it sort of does in AddRegisterOperand when it adds the
extra
>> COPY.... not sure why it does this instead of just checking the
original MI
>> and seeing if the reg class needed is in the list and then just
changing
>> the vreg reg class for the original MI, that seems like a better
solution?)
>> It's like it's actually picking some reg class first and then
trying to fix
>> it's error by adding MORE instructions instead of finding the right
reg
>> class the first time.
>>
>> Thanks.
>>
>> On Wed, Aug 19, 2015 at 1:32 PM, Quentin Colombet <qcolombet at
apple.com>
>> wrote:
>>
>>>
>>> On Aug 19, 2015, at 9:42 AM, Ryan Taylor <ryta1203 at
gmail.com> wrote:
>>>
>>> It seems the problem arises from using multiple reg classes for one
MI
>>> in the td file, I guess.
>>>
>>>
>>> Probably, that does not sound something used widely :).
>>>
>>>
>>> I'm not sure it takes first available, if I swap the reg
classes in the
>>> list it does not change and if I replace the GPR reg class with
something
>>> different than it picks the base reg class fine, potentially it is
using
>>> the reg class with most available? idk.
>>>
>>>
>>> My guess is it would take the register class that come first in the
>>> register class IDs (not the list on the instruction itself), but I
am just
>>> guessing.
>>>
>>>
>>> I just need to create MIs for every possible case I guess.
>>> Thanks for the help! :)
>>>
>>> On Wed, Aug 19, 2015 at 12:04 PM, Quentin Colombet <qcolombet at
apple.com>
>>> wrote:
>>>
>>>> Hi Ryan,
>>>>
>>>> On Aug 19, 2015, at 6:35 AM, Ryan Taylor via llvm-dev <
>>>> llvm-dev at lists.llvm.org> wrote:
>>>>
>>>> Essentially it doesn't appear that the reg class assignment
is based on
>>>> uses and is instead inserting an extra COPY for this. Is this
accurate? If
>>>> so, why?
>>>>
>>>>
>>>> We match the instructions bottom-up and I believe that copy are
>>>> automatically inserted when the register classes do not match.
>>>> That seems strange to me that the isel logs are exactly the
same but
>>>> still you are seeing a different result of instruction
selection.
>>>>
>>>>
>>>> In this above example, I'm getting an extra "mov %r0,
$b1" (this is an
>>>> MI::COPY) even though "mov @a, %b1" (this is an
MI::MOV) is entirely
>>>> acceptable since both GPRRegs and BaseRegs are in the reg class
list..
>>>>
>>>> If the heuristic is simply picking the first available reg
class or the
>>>> reg class with the most available, for example, instead of
picking the reg
>>>> class based on uses, I will have to change this heuristic to
reduce code
>>>> bloat, or we'll have to add backend passes to reduce the
code bloat.
>>>>
>>>>
>>>> I think the current approach is rather simple and we just match
the
>>>> type for the first available reg class (assuming your selection
dag
>>>> patterns are not choosing something different). There are not
per say
>>>> passes to reduce this "code bloat” since we never
encounter this problem in
>>>> practice. The peephole optimizer has some logic to avoid this
move and
>>>> CodeGenPrepare also does a few transformation to avoid some of
that.
>>>>
>>>> Anyhow, I’d say debug the isel process (probably the
InstrEmitter part)
>>>> to see where the problem arise.
>>>>
>>>> Cheers,
>>>> -Quentin
>>>>
>>>>
>>>> Thanks.
>>>>
>>>> On Mon, Aug 17, 2015 at 7:00 PM, Ryan Taylor <ryta1203 at
gmail.com>
>>>> wrote:
>>>>
>>>>>
>>>>> ---------- Forwarded message ----------
>>>>> From: Ryan Taylor <ryta1203 at gmail.com>
>>>>> Date: Mon, Aug 17, 2015 at 6:59 PM
>>>>> Subject: Re: [LLVMdev] TableGen Register Class not matching
for MI in
>>>>> 3.6
>>>>> To: Quentin Colombet <qcolombet at apple.com>
>>>>> Cc: "llvmdev at cs.uiuc.edu" <llvmdev at
cs.uiuc.edu>
>>>>>
>>>>>
>>>>> The isel logs are exactly the same, nothing changed,
it's matching the
>>>>> same instructions just the reg classes are different.
>>>>>
>>>>> Where in the SelectionDAG is the code that adds an extra MI
COPY? I'll
>>>>> have to set a breakpoint and debug backwards from there to
see what's going
>>>>> on. It's only happening with the GPRRegs class, for
example if I use
>>>>> another reg class instead along with the base regs than it
matches the base
>>>>> reg just fine, it's like GPR is overriding any other
reg class matching.
>>>>>
>>>>> Thanks.
>>>>>
>>>>> On Fri, Jul 31, 2015 at 1:21 PM, Quentin Colombet
<qcolombet at apple.com
>>>>> > wrote:
>>>>>
>>>>>>
>>>>>> On Jul 31, 2015, at 10:14 AM, Ryan Taylor <ryta1203
at gmail.com> wrote:
>>>>>>
>>>>>> Quentin,
>>>>>>
>>>>>>  It's in the instruction selection, sorry I forgot
to mention that.
>>>>>> The Vreg class is GPR and an extra COPY is generated to
copy from the GPR
>>>>>> to the Base Reg, even though my 'mov'
instruction has Base in the Register
>>>>>> class list.
>>>>>>
>>>>>>
>>>>>> Then, I suggest you use -debug-only=isel to check what
changed in
>>>>>> your selection instruction process. I do not see
off-hand what could have
>>>>>> caused.
>>>>>>
>>>>>> Sorry for not being more helpful here.
>>>>>>
>>>>>> Q.
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Fri, Jul 31, 2015 at 12:50 PM, Quentin Colombet <
>>>>>> qcolombet at apple.com> wrote:
>>>>>>
>>>>>>> Hi Ryan,
>>>>>>>
>>>>>>> Could you check where those moves come from?
>>>>>>>
>>>>>>> In particular, is this the product of the
instruction selection
>>>>>>> process?
>>>>>>>
>>>>>>> You use -print-machineinstrs to see when it is
inserted.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> -Quentin
>>>>>>>
>>>>>>> > On Jul 30, 2015, at 2:02 PM, Ryan Taylor
<ryta1203 at gmail.com>
>>>>>>> wrote:
>>>>>>> >
>>>>>>> > In LLVM 3.6,
>>>>>>> >
>>>>>>> >  We have an instruction that uses a register
class that is defined
>>>>>>> of several different reg classes. In 3.4 this works
fine but in 3.6 this is
>>>>>>> broken.
>>>>>>> >
>>>>>>> > For example, I have a mov instruction. mov can
be executed between
>>>>>>> different register types (ie gpr, index, base,
etc..)
>>>>>>> >
>>>>>>> > In 3.4, we would get something like this:
>>>>>>> >
>>>>>>> > mov @a, %b1 // moving this immediate to a base
register, which is
>>>>>>> what we want
>>>>>>> >
>>>>>>> > In 3.6, we now get this:
>>>>>>> >
>>>>>>> > mov @a, %r0  // r0 = gpr
>>>>>>> > mov %r0, %b1 // b1 = base reg
>>>>>>> >
>>>>>>> > The register class looks like this:
>>>>>>> >
>>>>>>> > def ARegs : RegisterClass<"us",
[i16], i16, (add GPRRegs,
>>>>>>> IndexRegs, BaseRegs)>;
>>>>>>> >
>>>>>>> > I have absolutely no idea why this is not
matching any longer?
>>>>>>> >
>>>>>>> > The fix here is to define an MI with explicit
single register
>>>>>>> class (ie it only allows PTRRegs as the
destination).
>>>>>>> >
>>>>>>> > This must be an issue with something else and
not the tablegen but
>>>>>>> if that was the case I'm not sure. Anyway help
would be great, what should
>>>>>>> I be looking at here?
>>>>>>> >
>>>>>>> > Thanks.
>>>>>>> >
_______________________________________________
>>>>>>> > LLVM Developers mailing list
>>>>>>> > LLVMdev at cs.uiuc.edu        
http://llvm.cs.uiuc.edu
>>>>>>>
<https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.cs.uiuc.edu_&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=7VQTHNyB_IcF4I86741RzVduPmdgCRL_mHUfuUmnL0Y&s=fK8KXlXNFjX9EyldRbiY4GwaxZBFjvTYEXwRJuFLJvg&e=>
>>>>>>> >
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>>>>>
<https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.cs.uiuc.edu_mailman_listinfo_llvmdev&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=7VQTHNyB_IcF4I86741RzVduPmdgCRL_mHUfuUmnL0Y&s=TAPJBdWJfY-53g2FEN-kOKTo2nDJxcpEGNpqpgcrN9U&e=>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> llvm-dev at lists.llvm.org
>>>>
>>>>
https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Ddev&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=7VQTHNyB_IcF4I86741RzVduPmdgCRL_mHUfuUmnL0Y&s=N0TOaZZSrDVRpbh_uMG5DV5ZZ2_KVMcBWtK9vGIaByY&e>>>>
>>>>
>>>>
>>>
>>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150824/b0a6711d/attachment.html>

> On Aug 24, 2015, at 1:30 PM, Ryan Taylor <ryta1203 at gmail.com>
wrote:
> 
> I'm trying to do something like this:
> 
> // Dst = NewVReg's reg class
> // *II = MCInstrDesc
> // IIOpNum = II Operand Num
> 
> if (TRI->getCommonSubClass(DstRC,
TRI->getRegClass(II->OpInfo[IIOpNum].RegClass)) == DstRC)
>      MRI->setRegClass(VReg, DstRC);
> else
>      BuildMI(... TargetOpcode::COPY...)
> 
> The condition is trying to reset the reg class if the DstRC reg class is
valid for the operand num of the machine instruction. If the NewVReg register
class is not valid for that operand of the machine instruction I want to
generate a COPY instruction (as it does now all the time). This condition should
check for intersection, right?
Yes, that’s right.
> It should find the common subclass of the new register class and the valid
reg class of the operand for that machine instruction.
> 
> Unfortunately it looks like that condition is always being set to true (or
I'm getting a TON of false positives).
Where does II come from?
From the snippet, I am guessing it is the instruction that uses NewVReg, i.e.,
you are checking that the class for NewVReg matches the class for NewVReg… which
by construction is always true!

You want to check "common subclass” of DstRC and SrcRC.

Cheers,
Q.
> 
> Thanks.
> 
> On Mon, Aug 24, 2015 at 2:09 PM, Quentin Colombet <qcolombet at
apple.com <mailto:qcolombet at apple.com>> wrote:
> 
>> On Aug 22, 2015, at 9:10 AM, Ryan Taylor <ryta1203 at gmail.com
<mailto:ryta1203 at gmail.com>> wrote:
>> 
>> One last question regarding this please.
>> 
>> Why aren't we simply changing the register class in
AddRegisterOperand instead of building a new COPY? I admit I haven't thought
this out but for my test cases so far this works just fine and reduces the
number of ASM mov instructions that are being produced.
>> 
>> For example, instead of BuildMI(..., TII->get(TargetOpcode::COPY),
NewVReg).addReg(VReg), use something like MRI->setRegClass(VReg,
MRI->getRegClass(NewVReg)) ?
> 
> The problem is that the old register class and the new one may not
intersect. I do not know exactly what makes us create a new vreg w.r.t., but
probably if the class is not identical we create one. You can try to use
contraintsRegClass to get the intersection.
> 
> Q.
> 
>> 
>> What is the reasoning behind adding the extra instruction instead of
changing the reg class?
>> 
>> On Wed, Aug 19, 2015 at 2:04 PM, Ryan Taylor <ryta1203 at gmail.com
<mailto:ryta1203 at gmail.com>> wrote:
>> Yes, you're probably right about the ID. The odd part is that I
have other simpler instructions that use the same type of superset and it
always, so far, matches correctly (it doesn't just pick GPRRegs all the
time).
>> 
>> Like I said, we can just 'fill in the gaps' with new MIs but
that sure seems like a brush off solution. The td files would be so much cleaner
if you could have a superset reg class that actually matched the correct reg
usage (which it sort of does in AddRegisterOperand when it adds the extra
COPY.... not sure why it does this instead of just checking the original MI and
seeing if the reg class needed is in the list and then just changing the vreg
reg class for the original MI, that seems like a better solution?) It's like
it's actually picking some reg class first and then trying to fix it's
error by adding MORE instructions instead of finding the right reg class the
first time.
>> 
>> Thanks.
>> 
>> On Wed, Aug 19, 2015 at 1:32 PM, Quentin Colombet <qcolombet at
apple.com <mailto:qcolombet at apple.com>> wrote:
>> 
>>> On Aug 19, 2015, at 9:42 AM, Ryan Taylor <ryta1203 at gmail.com
<mailto:ryta1203 at gmail.com>> wrote:
>>> 
>>> It seems the problem arises from using multiple reg classes for one
MI in the td file, I guess.
>> 
>> Probably, that does not sound something used widely :).
>> 
>> 
>>> I'm not sure it takes first available, if I swap the reg
classes in the list it does not change and if I replace the GPR reg class with
something different than it picks the base reg class fine, potentially it is
using the reg class with most available? idk.
>> 
>> My guess is it would take the register class that come first in the
register class IDs (not the list on the instruction itself), but I am just
guessing.
>> 
>>> 
>>> I just need to create MIs for every possible case I guess.
>>> Thanks for the help! :)
>>> 
>>> On Wed, Aug 19, 2015 at 12:04 PM, Quentin Colombet <qcolombet at
apple.com <mailto:qcolombet at apple.com>> wrote:
>>> Hi Ryan,
>>> 
>>>> On Aug 19, 2015, at 6:35 AM, Ryan Taylor via llvm-dev
<llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>>
wrote:
>>>> 
>>>> Essentially it doesn't appear that the reg class assignment
is based on uses and is instead inserting an extra COPY for this. Is this
accurate? If so, why?
>>> 
>>> We match the instructions bottom-up and I believe that copy are
automatically inserted when the register classes do not match.
>>> That seems strange to me that the isel logs are exactly the same
but still you are seeing a different result of instruction selection.
>>> 
>>>> 
>>>> In this above example, I'm getting an extra "mov %r0,
$b1" (this is an MI::COPY) even though "mov @a, %b1" (this is an
MI::MOV) is entirely acceptable since both GPRRegs and BaseRegs are in the reg
class list..
>>>> 
>>>> If the heuristic is simply picking the first available reg
class or the reg class with the most available, for example, instead of picking
the reg class based on uses, I will have to change this heuristic to reduce code
bloat, or we'll have to add backend passes to reduce the code bloat.
>>> 
>>> I think the current approach is rather simple and we just match the
type for the first available reg class (assuming your selection dag patterns are
not choosing something different). There are not per say passes to reduce this
"code bloat” since we never encounter this problem in practice. The
peephole optimizer has some logic to avoid this move and CodeGenPrepare also
does a few transformation to avoid some of that.
>>> 
>>> Anyhow, I’d say debug the isel process (probably the InstrEmitter
part) to see where the problem arise.
>>> 
>>> Cheers,
>>> -Quentin
>>> 
>>>> 
>>>> Thanks.
>>>> 
>>>> On Mon, Aug 17, 2015 at 7:00 PM, Ryan Taylor <ryta1203 at
gmail.com <mailto:ryta1203 at gmail.com>> wrote:
>>>> 
>>>> ---------- Forwarded message ----------
>>>> From: Ryan Taylor <ryta1203 at gmail.com <mailto:ryta1203
at gmail.com>>
>>>> Date: Mon, Aug 17, 2015 at 6:59 PM
>>>> Subject: Re: [LLVMdev] TableGen Register Class not matching for
MI in 3.6
>>>> To: Quentin Colombet <qcolombet at apple.com
<mailto:qcolombet at apple.com>>
>>>> Cc: "llvmdev at cs.uiuc.edu <mailto:llvmdev at
cs.uiuc.edu>" <llvmdev at cs.uiuc.edu <mailto:llvmdev at
cs.uiuc.edu>>
>>>> 
>>>> 
>>>> The isel logs are exactly the same, nothing changed, it's
matching the same instructions just the reg classes are different.
>>>> 
>>>> Where in the SelectionDAG is the code that adds an extra MI
COPY? I'll have to set a breakpoint and debug backwards from there to see
what's going on. It's only happening with the GPRRegs class, for example
if I use another reg class instead along with the base regs than it matches the
base reg just fine, it's like GPR is overriding any other reg class
matching.
>>>> 
>>>> Thanks.
>>>> 
>>>> On Fri, Jul 31, 2015 at 1:21 PM, Quentin Colombet <qcolombet
at apple.com <mailto:qcolombet at apple.com>> wrote:
>>>> 
>>>>> On Jul 31, 2015, at 10:14 AM, Ryan Taylor <ryta1203 at
gmail.com <mailto:ryta1203 at gmail.com>> wrote:
>>>>> 
>>>>> Quentin,
>>>>> 
>>>>>  It's in the instruction selection, sorry I forgot to
mention that. The Vreg class is GPR and an extra COPY is generated to copy from
the GPR to the Base Reg, even though my 'mov' instruction has Base in
the Register class list.
>>>> 
>>>> Then, I suggest you use -debug-only=isel to check what changed
in your selection instruction process. I do not see off-hand what could have
caused.
>>>> 
>>>> Sorry for not being more helpful here.
>>>> 
>>>> Q.
>>>> 
>>>>> 
>>>>> 
>>>>> On Fri, Jul 31, 2015 at 12:50 PM, Quentin Colombet
<qcolombet at apple.com <mailto:qcolombet at apple.com>> wrote:
>>>>> Hi Ryan,
>>>>> 
>>>>> Could you check where those moves come from?
>>>>> 
>>>>> In particular, is this the product of the instruction
selection process?
>>>>> 
>>>>> You use -print-machineinstrs to see when it is inserted.
>>>>> 
>>>>> Thanks,
>>>>> -Quentin
>>>>> 
>>>>> > On Jul 30, 2015, at 2:02 PM, Ryan Taylor <ryta1203
at gmail.com <mailto:ryta1203 at gmail.com>> wrote:
>>>>> >
>>>>> > In LLVM 3.6,
>>>>> >
>>>>> >  We have an instruction that uses a register class
that is defined of several different reg classes. In 3.4 this works fine but in
3.6 this is broken.
>>>>> >
>>>>> > For example, I have a mov instruction. mov can be
executed between different register types (ie gpr, index, base, etc..)
>>>>> >
>>>>> > In 3.4, we would get something like this:
>>>>> >
>>>>> > mov @a, %b1 // moving this immediate to a base
register, which is what we want
>>>>> >
>>>>> > In 3.6, we now get this:
>>>>> >
>>>>> > mov @a, %r0  // r0 = gpr
>>>>> > mov %r0, %b1 // b1 = base reg
>>>>> >
>>>>> > The register class looks like this:
>>>>> >
>>>>> > def ARegs : RegisterClass<"us", [i16],
i16, (add GPRRegs, IndexRegs, BaseRegs)>;
>>>>> >
>>>>> > I have absolutely no idea why this is not matching any
longer?
>>>>> >
>>>>> > The fix here is to define an MI with explicit single
register class (ie it only allows PTRRegs as the destination).
>>>>> >
>>>>> > This must be an issue with something else and not the
tablegen but if that was the case I'm not sure. Anyway help would be great,
what should I be looking at here?
>>>>> >
>>>>> > Thanks.
>>>>> > _______________________________________________
>>>>> > LLVM Developers mailing list
>>>>> > LLVMdev at cs.uiuc.edu <mailto:LLVMdev at
cs.uiuc.edu>         http://llvm.cs.uiuc.edu
<https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.cs.uiuc.edu_&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=7VQTHNyB_IcF4I86741RzVduPmdgCRL_mHUfuUmnL0Y&s=fK8KXlXNFjX9EyldRbiY4GwaxZBFjvTYEXwRJuFLJvg&e=>
>>>>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
<https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.cs.uiuc.edu_mailman_listinfo_llvmdev&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=7VQTHNyB_IcF4I86741RzVduPmdgCRL_mHUfuUmnL0Y&s=TAPJBdWJfY-53g2FEN-kOKTo2nDJxcpEGNpqpgcrN9U&e=>
>>>>> 
>>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>>
https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Ddev&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=7VQTHNyB_IcF4I86741RzVduPmdgCRL_mHUfuUmnL0Y&s=N0TOaZZSrDVRpbh_uMG5DV5ZZ2_KVMcBWtK9vGIaByY&e=
<https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Ddev&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=7VQTHNyB_IcF4I86741RzVduPmdgCRL_mHUfuUmnL0Y&s=N0TOaZZSrDVRpbh_uMG5DV5ZZ2_KVMcBWtK9vGIaByY&e=>
>>> 
>>> 
>> 
>> 
>> 
> 
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150824/8a6ec6ef/attachment.html>