rdaly525 / coreir

BSD 3-Clause "New" or "Revised" License
100 stars 24 forks source link

slicing breaks inlining #895

Closed rdaly525 closed 4 years ago

rdaly525 commented 4 years ago

@leonardt the new slicing syntax breaks the current inlining code and any code that uses passthroughs.(https://github.com/rdaly525/coreir/blob/master/src/ir/inline.cpp). In particular the case when there are non-matching slices for a bitvector port in the module instantiation and the module definition.

Unfortunately this is quite a complicated fix that requires instantiating coreir.wire nodes when this case arises.

A few fixes in the interim: 1) Disallow inlining when there are slices 2) Always translate slices to single bit selects before inlining

Let me know if this makes sense.

leonardt commented 4 years ago

Do you have a test to reproduce the issue?

rdaly525 commented 4 years ago

@leonardt creating a simple example now

rdaly525 commented 4 years ago
{"top":"global.A",
  "namespaces": {
    "global": {
      "modules": {
        "A": {
          "type": ["Record",[
            ["in", ["Array",16,"BitIn"]],
            ["out", ["Array",16,"Bit"]]
          ]],
            "instances": {
            "i0": {"modref": "global.B"}
          },
          "connections": [
            ["self.in.0:3", "i0.in.0:3"],
            ["self.in.3:16", "i0.in.3:16"],
            ["i0.out", "self.out"]
          ]
        },
        "B": {
          "type": ["Record",[
            ["in", ["Array",16,"BitIn"]],
            ["out", ["Array",16,"Bit"]]
          ]],
          "instances": {
            "n1": {
              "genref": "coreir.neg",
              "genargs": {"width":["Int", 16]}
            }
          },
          "connections": [
            ["self.in.0:10","n1.in.0:10"],
            ["self.in.10:16","n1.in.10:16"],
            ["n1.out","self.out"]
          ]
        }
      }
    }
  }
}

running flatten on this: >coreir -i example.json -p flatten

will produce:

{"top":"global.A",
"namespaces":{
  "global":{
    "modules":{
      "A":{
        "type":["Record",[
          ["in",["Array",16,"BitIn"]],
          ["out",["Array",16,"Bit"]]
        ]],
        "instances":{
          "i0$n1":{
            "genref":"coreir.neg",
            "genargs":{"width":["Int",16]}
          }
        },
        "connections":[
          ["self.out","i0$n1.out"]
        ]
      },
      "B":{
        "type":["Record",[
          ["in",["Array",16,"BitIn"]],
          ["out",["Array",16,"Bit"]]
        ]],
        "instances":{
          "n1":{
            "genref":"coreir.neg",
            "genargs":{"width":["Int",16]}
          }
        },
        "connections":[
          ["self.in.0:10","n1.in.0:10"],
          ["self.in.10:16","n1.in.10:16"],
          ["self.out","n1.out"]
        ]
      }
    }
  }
}
}
rdaly525 commented 4 years ago

Note that the inline code does not appropriately handle the connections between A.in and i0$n1.in. I think the "correct" way for this type of situation to be resolved is to have a coreir.wire instantiated between self.in and the i0$n1.in.

leonardt commented 4 years ago

Thanks will take a look

leonardt commented 4 years ago

Closed by #897