[llvm-commits] [llvm] r155364 - in /llvm/trunk: include/llvm/CodeGen/DFAPacketizer.h lib/CodeGen/DFAPacketizer.cpp

[Sirish Pande]

On 4/24/2012 10:58 AM, Andrew Trick wrote:
>
> On Apr 24, 2012, at 7:54 AM, Sirish Pande <spande at codeaurora.org 
> <mailto:spande at codeaurora.org>> wrote:
>
>> On 4/23/2012 3:26 PM, Andrew Trick wrote:
>>>
>>> On Apr 23, 2012, at 11:55 AM, Sirish Pande <spande at codeaurora.org 
>>> <mailto:spande at codeaurora.org>> wrote:
>>>
>>> +  VLIWScheduler->exitRegion();
>>>
>>> I'm not sure why you would do this *before* packetizing. It's ok if 
>>> it works, just a little misleading. You should probably add a call 
>>> to VLIWScheduler->finishBlock() somewhere.
>>
>> Thanks for pointing out about the finishBlock(). I will add 
>> finishBlock() at the end of PacketizeMI function.
>>
>> You probably know that we are getting the dependence information from 
>> the scheduler. Once we have the dependence info, along with the DFA 
>> resource manager, we can packetize the way our architecture prefers.  
>> We are not using the scheduler to *schedule* instructions during 
>> packetization. Hence, we have enterRegion() and exitRegion() around 
>> schedule. Esssentially, this logic is borrowed from around line 200 
>> of MachineScheduler.cpp
>
> After callling ScheduleDAGInstrs::exitRegion, no one should refer DAG 
> for that region (it happens to work, it's just a minor abuse). 
> Likewise, after calling finishBlock no one should ask for the DAG's 
> current block. I only mention this because others are learning the API 
> from your client.

Let me not abuse DAG then. Let me try to put call the exitRegion() 
towards the end, run our hexagon test cases. If everything works well 
(which I believe should), I will make sure that after exitRegion, I do 
not refer DAG.


>
>>>
>>> +  // Generate MI -> SU map.
>>> +  //std::map <MachineInstr*, SUnit*> MIToSUnit;
>>> +  MIToSUnit.clear();
>>> +  for (unsigned i = 0, e = VLIWScheduler->SUnits.size(); i != e; ++i) {
>>> +    SUnit *SU = &VLIWScheduler->SUnits[i];
>>> +    MIToSUnit[SU->getInstr()] = SU;
>>> +  }
>>>
>>> I added ScheduleDAGInstrs::MISUnitMap so you wouldn't need your own. 
>>> I haven't tested it with a PostRA scheduler, but would like to know 
>>> if it doesn't work.
>>
>> Have you checked this functionality in? I don't see it.
>
> ScheduleDAGInstrs::initSUnits populates the map. I'm not sure why that 
> wouldn't work for you. Note that the ExitSU is not mapped, but you're 
> not really using ExitSU anyway.

Is it possible that you make initSUnits public?  Since its protected - I 
have to juggle around to access that function.

>
> -Andy
>


-- 
Qualcomm Innovation Center, Inc is a member of Code Aurora Forum

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120424/c87988cb/attachment.html>