make clean conflicts with per action "user IP"

[david-mle]

Hello,
the removal operations of make clean try to remove a directory that potentially contains "user IP" in the form of TCL files. These are to be considered source code and may not be removed.
Currently however, make clean fails to remove them because the rm target is a directory and the rm parameters don't allow directory removal...
Having user IP TCL files in a directory called VHDL is strange anyhow :) Maybe its name should be changed and the remove operation should be dropped from make clean?
Am I not understanding the concept of this hw/vhdl directory or of user IP correctly?

snap/hardware/Makefile

Line 479 in 7b6306a

@$(RM) $(ACTION_ROOT)/hw/vhdl

snap/hardware/setup/create_ip.tcl

Lines 501 to 505 in 7b6306a

	# User IPs
	set action_vhdl $action_root/hw/vhdl
	if { [file exists $action_vhdl] == 1 } {
	set tcl_exists [exec find $action_vhdl/ -name *.tcl]

[CLEAN ENVIRONMENT...] start 16:20:24 Fri Aug 17 2018
vivado project
IPs
sim files
log files
action / application
rm: cannot remove ‘...../actions/...../hw/vhdl’: Is a directory
make[1]: *** [clean] Error 1
make[1]: Leaving directory `....../hardware'
make: *** [menuconfig] Error 2

[boekholt]

@david-mle Yes, you are right, as of today this looks strange, indeed.
The actions/.../hw/vhdl link got introduced for HLS actions specifically. At that point in time we only had examples for HLS actions that did not include IP generated by the HLS process.
When we got to the point working with HLS action examples containing IP it turned out that Vivado is storing the generated tcl scripts in the directory that we pointed to with the vhdl link.
So, maybe we should rename that link.

Regarding your problem: I am surprised about the message

rm: cannot remove ‘...../actions/...../hw/vhdl’: Is a directory

because that should be a link (as described above). This link is usually generated by the script
https://github.com/open-power/snap/blob/master/actions/hls.mk

Did you create that directory manually?

That would also be a good reason for renaming that link which got introduced for convenience ("vhdl" really is a name that would be straightforward for a directory containing all vhdl sources belonging to the action).

[david-mle]

Hello @boekholt ,
I'm building an HDL action which requires some Xilinx IP cores. I found the user IP features in the TCL scripts and from that concluded the expected directory structure. Yes, I did create the vhdl directory manually (as a directory not symlink) as I did not notice it gets generated automatically as a link in the HLS case. I searched for the vhdl folder in the examples and concluded this feature is currently not used. I missed the generator script.

I currently have no good understanding of the SNAP HLS flow, but can look into it next week. Maybe we can find a way for user IP in actions that fits both flows without too many conditional branches in the build system.

Thanks for looking into this, David

[boekholt]

Hi @david-mle,
I created the branch hls_src_link and pull request #813 to address this issue. Please have a look and let me know if that is the resolving your issues.

Thanks
Sven

[david-mle]

Hello @boekholt ,

the changes look good to me as far as naming and make clean is concerned. They however still don't allow me to have user IP within HDL actions. This should probably a separate commit anyways.
I have a suggestion for such user IP integration building on top of your branch:
https://github.com/david-mle/snap/commits/hls_src_link
After having done the changes I noticed that similar work seems to be in progress here:
https://github.com/open-power/snap/commits/hdl_helloworld_gen_ip
Either way I think it would be good to support user IP for HDL actions.

Best regards, David

[david-mle]

Oh, one more thing, my commit is tested with HDL actions without user IP (HDL Example), a HDL action with user IP, and with HLS Intersect. HLS Intersect has VHDL files for IP generated but no user IP TCL files. Do you know which HLS action to use to test user IP TCL scripts?

[boekholt]

@david-mle If I am remembering it correctly, there are HLS actions that make use of user IP. But they are not on github. I need to ask around.