parameters_append overwrite inherited parameters_append

[hipolitoguzman]

Hi! First of all, thanks for this awesome tool! It's reducing a lot of friction in my day-to day development, and being able to finally have a sane way of setting up continuous integration is priceless :D

I'm having an issue that I'm not sure if it's the expected behavior. When I use parameters_append more than once, the previous inherited parameters_append properties get overwritten by the last one.

In the MWE below, target default defines PARAM1 (with parameters), target child (which inherits from default) defines PARAM2 (with parameters_append) and target grandchild (which inherits from child) defines PARAM3 (with parameters_append). When setting up target grandchild, PARAM2 doesn't appear in the generated tcl scripts.

Since using parameters in a target would already override all inherited parameters, I would expect parameters_append to allow appending a number of parameters among different inheritance steps.

I suppose the overwriting is happening at the YAML level, nevertheless it would be nice to have a way of having multiple parameters_append, to avoid having duplicated constants.

How to reproduce

1. Copy the following YAML code into a file named test_parameters_append.core

CAPI=2:                                                                         
name: ::test_parameters_append:0.1                                              
description: Testing whether a second parameters_append overwrites the first one

targets:                                                                        
  default: &default                                                             
    description: default target, includes "parameters" property                 
    default_tool: ise                                                           
    toplevel: this_entity_doesnt_exist                                          
    parameters: [PARAM1=1]                                                      

  child: &child                                                                 
    <<: *default                                                                
    description: inherits from default, has "parameters_append" property        
    tools:                                                                      
      ise:                                                                      
       family: Spartan6                                                         
       device: xc6slx9                                                          
       package: csg324                                                          
       speed: -2                                                                
    parameters_append: [PARAM2=2]                                               

  grandchild:                                                                   
    <<: *child                                                                  
    description: inherits from child, has "parameters_append" property          
    parameters_append: [PARAM3=3]                                               

parameters:                                                                     
  PARAM1:                                                                       
    datatype    : int                                                           
    description : First parameter                                               
    paramtype   : generic                                                       
  PARAM2:                                                                       
    datatype    : int                                                           
    description : Second parameter                                              
    paramtype   : generic                                                       
  PARAM3:                                                                       
    datatype    : int                                                           
    description : Third parameter                                               
    paramtype   : generic              

2. Have Xilinx ISE in your $PATH (the issue also happens with Vivado, so Vivado could also be used)

3. Run the command fusesoc --verbose --cores-root . run --setup --target=grandchild ::test_parameters_append:0.1

What I expected to happen

I expected the generated tcl file at build/test_parameters_append_0.1/grandchild-ise/test_parameters_append_0.1.tcl to contain the following line:

project set "Generics, Parameters" "PARAM1=1|PARAM2=2|PARAM3=3" -process "Synthesize - XST"

What actually happened

Actually, PARAM2 is missing from the generated tcl file:

project set "Generics, Parameters" "PARAM1=1|PARAM3=3" -process "Synthesize - XST"

[sifferman]

I believe this is true for all keys with type list (filesets, generate, parameters, vpi): https://fusesoc.readthedocs.io/en/stable/ref/capi2.html#list

I agree that it should be changed as well, but I am a bit worried it may break many people's core files. (I know it'll break a couple of mine)

I believe this is the code that should be modified to change the functionality: https://github.com/olofk/fusesoc/blob/fc3456da747cac02b586679a9436491102bf32a5/fusesoc/capi2/coredata.py#L63-L89

[olofk]

Hola @hipolitoguzman, y gracias por tus palabras símpaticas!

I can see now that the behavior that you were expecting makes sense, but as @sifferman says, it's not implemented like that unfortunately. Mainly because I just never thought of it. I'm not sure if this is of any help, but parameters are appended from dependencies, so if that's an option, you could split these out into separate cores instead.

[hipolitoguzman]

Thank you @sifferman and @olofk for your explanations!

I suppose then that this would be more elegantly solved in the future with variable substitutions (which I think are on the roadmap).

Another option would be to support something like parameters_append*, but this could probably get very messy real fast, and end up making core files less readable.

For my specific use case I can manage by changing where I define the parameters, so I can work around this.

Let me know if I should close this issue, and thanks again