Fixed file naming issues when having multiple instances of an IP #58
Conversation
There was a problem hiding this comment.
Thank you for the pull request! All of the issues are valid and we would like to get them fixed. There are some changes requested (see comments). One set of changes around addressing the PS in pblock creation for Zynq devices we should revert for now and we'll work on a more holistic solution that will satisfy all scenarios more robustly.
| @@ -68,7 +68,7 @@ | |||
|
|
|||
| /** X dimension over Y dimension */ | |||
| public float ASPECT_RATIO = 0.125f;//1.5f; | |||
| public float OVERHEAD_RATIO = 1.5f;//1.25f; | |||
| public float OVERHEAD_RATIO = 4.5f;//1.25f; | |||
There was a problem hiding this comment.
@ElaHobby - I think we probably want to keep the OVERHEAD_RATIO default to 1.5. I'm sure you have found instances where it has needed to be increased, but we probably aren't ready to change it for everyone.
| } | ||
| if(numBRAMColumns > 0){ | ||
| int pIdx = 0; | ||
| for(int i=0; pIdx < p.size(); i++){ | ||
| TileTypeEnum t = dev.getTile(row, col+i).getTileTypeEnum(); | ||
| if(Utils.isBRAM(t)){ | ||
| for(Site s : dev.getTile(row, col+i).getSites()){ | ||
| if(s.getSiteTypeEnum() == SiteTypeEnum.RAMBFIFO36) upperLeft = s; | ||
| if((s.getSiteTypeEnum() == SiteTypeEnum.RAMBFIFO36)||(s.getSiteTypeEnum() == SiteTypeEnum.RAMBFIFO36E1)) upperLeft = s; // Update. Goal: support for 7 series |
There was a problem hiding this comment.
Thanks, this change looks good. I will follow up later with a better check for BRAMs.
| @@ -940,16 +940,57 @@ public static void main_testing(String[] args) { | |||
| } | |||
| if(p.get(pIdx) == t) pIdx++; | |||
| } | |||
| sb.append("SLICE_X" + upperLeft.getInstanceX() + "Y" + (upperLeft.getInstanceY()-(numSLICERows-1)) + | |||
| ":SLICE_X" + (upperLeft.getInstanceX()+(numSLICEColumns+numSLICEMColumns)-1) + "Y" + upperLeft.getInstanceY()); | |||
|
|
|||
There was a problem hiding this comment.
This update is clearly attempting to address the issue of the pblock colliding with the PS which is not a homogeneous--this is a problem. Since we have a number of situations similar to this, perhaps its best if we hold off on these point solutions and work together on a more holistic approach. I think in this current state, it may break solutions for other devices.
| # Updated. Goal: avoid naming problems caused by the fact that the files copied into IP_CACHE have different names as the ones expected by RW. Error appears only in designs with multiple IPs with the same ID | ||
| # Line bellow removed and replaced by the following 2 lines | ||
| #read_xdc ${unzipDir}/${rootDcpFileName}_in_context.xdc | ||
| set file_name_xdc [glob -directory ${unzipDir} *_in_context.xdc] |
There was a problem hiding this comment.
This looks like it should work. I'm ok with this change, you can delete the old line as it stays in the history of git.
tcl/rapidwright.tcl
Outdated
| return $ip | ||
| } | ||
| } | ||
| # End of updated |
There was a problem hiding this comment.
Looks good, perhaps just add a newline at the end of the file.
The issue occurs in case of designs having more instances for an IP, when RW copies into IP_CACHE the already generated synthesis files. The order in which these files are copied determine the name of the .dcp files. This order does not correspond with the one that RW expects when generating opt runs.