Allow backend to extract incdir/defines on a per-sourcefile basis instead of per-core

[krishnan-gopal]

Fileset properties like incdir/defines that are specified for a particular fileset should not be merged with that of other filesets in the same core file. Currently, when the backend (Edalize) extracts the incdir/defines for a particular source-file, the resulting values for each file is the combined values of incdir/defines of all filesets within that same core file. It would be better to have these seperated. The solution is to seperate these values in fusesoc and edalize tools as shown:

fusesoc/edalizer.py To replace: Line 81 following:

_files = []
for file in core.get_files(_flags):
    if file.copyto:
        _name = file.copyto
        dst = os.path.join(work_root, _name)
        _dstdir = os.path.dirname(dst)
        if not os.path.exists(_dstdir):
            os.makedirs(_dstdir)
        shutil.copy2(os.path.join(files_root, file.name), dst)
    else:
        _name = os.path.join(rel_root, file.name)
    _files.append(
        {
            "name": str(_name),
            "core": str(core.name),
            "file_type": file.file_type,
            "is_include_file": file.is_include_file,
            "logical_name": file.logical_name,
        }
    )

snippet["files"] = _files

change to:

 _files = []
_filesets = []
for fileset in core.filesets.values():
    for file in fileset.files:
        if file.copyto:
            _name = file.copyto
            dst = os.path.join(work_root, _name)
            _dstdir = os.path.dirname(dst)
            if not os.path.exists(_dstdir):
                os.makedirs(_dstdir)
            shutil.copy2(os.path.join(files_root, file.name),
                        dst)
        else:
            _name = os.path.join(rel_root, file.name)
        _files.append({
            'name'            : _name,
            'file_type'       : file.file_type,
            'is_include_file' : file.is_include_file,
            'logical_name'    : file.logical_name})
    _filesets.append(_files)
 
snippet['filesets'] = _filesets

---

in edalize/edatool.py:

add following: to edatool.py in line 58:

self.filesets    = edam.get('filesets', [])

And to replace: Line 251 following:

incdirs = []
       src_files = []
       for f in self.files:
           if 'is_include_file' in f and f['is_include_file']:
               _incdir = os.path.dirname(f['name']) or '.'
               if force_slash:
                   _incdir = _incdir.replace('\\', '/')
               if not _incdir in incdirs:
                   incdirs.append(_incdir)
           else:
               _name = f['name']
               if force_slash:
                   _name = _name.replace('\\', '/')
               file_type = f.get('file_type', '')
               logical_name = f.get('logical_name', '')
               src_files.append(File(_name,
                                     file_type,
                                     logical_name))
       return (src_files, incdirs)

change to:

defines = []
incdirs = []
src_files = []
filesets = []
if not self.files:
    for fs in self.filesets:
        for f in fs:
            if 'is_include_file' in f and f['is_include_file']:
                _incdir = os.path.dirname(f['name']) or '.'
                if force_slash:
                    _incdir = _incdir.replace('\\', '/')
                if not _incdir in incdirs:
                    incdirs.append(_incdir)
            else:
                _name = f['name']
                if force_slash:
                    _name = _name.replace('\\', '/')
                file_type = f.get('file_type', '')
                logical_name = f.get('logical_name', '')
                defines = f['defines']
                src_files.append(File(_name,
                                    file_type,
                                    logical_name))
        filesets.append((src_files, incdirs, defines))
        incdirs = []
        src_files = []
        defines = []
    
    return filesets
else:
    for f in self.files:
        if 'is_include_file' in f and f['is_include_file']:
            _incdir = os.path.dirname(f['name']) or '.'
            if force_slash:
                _incdir = _incdir.replace('\\', '/')
            if not _incdir in incdirs:
                incdirs.append(_incdir)
        else:
            _name = f['name']
            if force_slash:
                _name = _name.replace('\\', '/')
            file_type = f.get('file_type', '')
            logical_name = f.get('logical_name', '')
            src_files.append(File(_name,
                                file_type,
                                logical_name))
    return (src_files, incdirs)

[olofk]

I see your point, but I think we need to consider two things

1. Is it really a problem, or is it ok to set these properties on all files in the core? So far I have not seen any code that breaks by getting additional incdirs or defines, but it sounds like you are having some design where this is a problem
2. Can we change this in a backwards compatible way? I believe there are many existing designs that will break with this change, e.g. if the include files are specified in a separate fileset.

I'm also not sure how you will set defines per fileset with this change. Defines (like other parameters such as vlogparam, generics, plusargs) are currently defined per target, so they would still be set for all filesets specified in that target

[krishnan-gopal]

here are my comments:

1. Is it really a problem, or is it ok to set these properties on all files in the core? So far I have not seen any code that breaks by getting additional incdirs or defines, but it sounds like you are having some design where this is a problem

In general, its rather confusing to read a compile script generated with all the defines and incdirs merged together, and compiling files with options that are not really needed. And then comes the scary unpredictability when two files with the same name but different content are in two different directories. I would vote for some more fine-grain control here.

2. Can we change this in a backwards compatible way? I believe there are many existing designs that will break with this change, e.g. if the include files are specified in a separate fileset.

I would be a fan of an implementation where you can specify these options at the file-set level (local options) as well as the core level (kind of like global options). This would result in a lot less repeated code in the .core file, and some cleaner hierarchical readibility.

[krishnan-gopal]

I recently realized that the include_path is a file-specific property in the CAPI2, which we can exploit for this need. What if we retain this property as file-specific even while getting the fileset files at the backend ? This would mean the File class in Edatool would have its own incdir, lets say file_specific_indir in addition to the exported incdirs which are universal. This would then allow every file to have its own incdir, which would also cover the issue #398

Thus we have 2 use-case scenarios:

1. The already implemented: We use is_include_file: true and optionally set include_path. This would treat the specified file as a directory, and we use that to export the specified directory into a list of incdirs which is universal across all files.

2. The potential enhancement: Proposed condition: If include_path is specified for a file with is_include_file: false This would treat the file as a fileset file whose include_path will not be exported to the universal list, but would rather contain this incdir as a property of that specific file. The resulting incdir list in the backend for each file will be the combination of the universally exported incdirs and the file-specific incdir for that file.

This would still keep the backwards compatibility intact (i.e universally exported incdirs), and also ensure that a wider variety of use-cases will be supported (accurate file-specific incdirs).

The condition under which this new scenario would be triggered is open for discussion.