From 2e06d45ca054ac6b04df88e52c5ed15160e80f1b Mon Sep 17 00:00:00 2001 From: Zachary Snow Date: Mon, 12 Jul 2021 13:44:56 -0400 Subject: [PATCH] fix inadvertent duplicate declaration generation Disabling the package item injection routine used in the enum conversion when there were no items to inject exposed cases where conversions would generate duplicate declarations. The hierarchical constant and param type conversions were trivially affected. The package conversion could inject class items within a generate region and then re-inject them outside of that generate region. The package conversions now uses an upgraded generate region flattening utility to ensure injected class items are seen. This also includes coverage for a conflict which occurred even without the enum conversion change. --- src/Convert/Enum.hs | 5 ++++- src/Convert/HierConst.hs | 5 ++++- src/Convert/Package.hs | 3 ++- src/Convert/ParamType.hs | 15 ++++++++------- src/Convert/Traverse.hs | 29 +++++++++++++++-------------- test/core/paramtype.sv | 1 + test/core/paramtype.v | 2 ++ 7 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/Convert/Enum.hs b/src/Convert/Enum.hs index 5934ed1..1641a0e 100644 --- a/src/Convert/Enum.hs +++ b/src/Convert/Enum.hs @@ -39,7 +39,10 @@ convertDescription :: Description -> [Description] convertDescription (description @ Part{}) = [Part attrs extern kw lifetime name ports items'] where - items' = inject enumItems items -- only keep what's used + items' = -- only keep what's used + if null enumItems + then items + else inject enumItems items Part attrs extern kw lifetime name ports items = description' (description', enumItems) = convertDescription' description convertDescription other = [other] diff --git a/src/Convert/HierConst.hs b/src/Convert/HierConst.hs index b4de6dc..3a2bd06 100644 --- a/src/Convert/HierConst.hs +++ b/src/Convert/HierConst.hs @@ -19,6 +19,7 @@ module Convert.HierConst (convert) where +import Control.Monad (when) import Data.Either (fromLeft) import qualified Data.Map.Strict as Map @@ -86,7 +87,9 @@ traverseExprM (expr @ (Dot _ x)) = do (Just ([_, _], _, Left{}), Just ([_, _], _, Left{})) -> return $ Ident x (Just (accesses @ [Access _ Nil, _], _, Left False), _) -> do - insertElem accesses (Left True) + details <- lookupElemM $ prefix x + when (details == Nothing) $ + insertElem accesses (Left True) return $ Ident $ prefix x (Just ([Access _ Nil, _], _, Left True), _) -> return $ Ident $ prefix x diff --git a/src/Convert/Package.hs b/src/Convert/Package.hs index 420708c..e82f12b 100644 --- a/src/Convert/Package.hs +++ b/src/Convert/Package.hs @@ -610,7 +610,8 @@ convertDescription pis (orig @ Part{}) = else Part attrs extern kw lifetime name ports items' where Part attrs extern kw lifetime name ports items = orig - items' = addItems pis Set.empty (map addUsedPIs items) + items' = addItems pis Set.empty $ map addUsedPIs $ + foldr breakGenerate [] items -- ensure decls are visible convertDescription _ other = other -- attempt to fix simple declaration order issues diff --git a/src/Convert/ParamType.hs b/src/Convert/ParamType.hs index c512d95..6cfc497 100644 --- a/src/Convert/ParamType.hs +++ b/src/Convert/ParamType.hs @@ -23,6 +23,7 @@ type Instance = Map.Map Identifier (Type, IdentSet) type Instances = Set.Set (Identifier, Instance) type IdentSet = Set.Set Identifier +type DeclMap = Map.Map Identifier Decl type UsageMap = [(Identifier, Set.Set Identifier)] convert :: [AST] -> [AST] @@ -241,20 +242,20 @@ typeIsUnresolved = collectUnresolvedExprM Dot {} = tell $ Any True collectUnresolvedExprM _ = return () -prepareTypeExprs :: Identifier -> Identifier -> Type -> (Type, (IdentSet, [Decl])) +prepareTypeExprs :: Identifier -> Identifier -> Type -> (Type, (IdentSet, DeclMap)) prepareTypeExprs instanceName paramName = runWriter . traverseNestedTypesM (traverseTypeExprsM $ traverseNestedExprsM prepareExpr) where - prepareExpr :: Expr -> Writer (IdentSet, [Decl]) Expr + prepareExpr :: Expr -> Writer (IdentSet, DeclMap) Expr prepareExpr (e @ Call{}) = do - tell (Set.empty, [decl]) + tell (Set.empty, Map.singleton x decl) prepareExpr $ Ident x where decl = Param Localparam (TypeOf e) x e x = instanceName ++ "_sv2v_pfunc_" ++ shortHash e prepareExpr (Ident x) = do - tell (Set.singleton x, []) + tell (Set.singleton x, Map.empty) return $ Ident $ paramName ++ '_' : x prepareExpr other = return other @@ -301,9 +302,9 @@ convertModuleItemM (orig @ (Instance m bindings x r p)) = bindingsMap = Map.fromList bindings resolvedTypesWithDecls = Map.mapMaybeWithKey resolveType bindingsMap resolvedTypes = Map.map (\(a, (b, _)) -> (a, b)) resolvedTypesWithDecls - addedDecls = concatMap (snd . snd . snd) $ - Map.toList resolvedTypesWithDecls - resolveType :: Identifier -> TypeOrExpr -> Maybe (Type, (IdentSet, [Decl])) + addedDecls = Map.elems $ Map.unions $ map (snd . snd) $ + Map.elems resolvedTypesWithDecls + resolveType :: Identifier -> TypeOrExpr -> Maybe (Type, (IdentSet, DeclMap)) resolveType _ Right{} = Nothing resolveType paramName (Left t) = Just $ prepareTypeExprs x paramName t diff --git a/src/Convert/Traverse.hs b/src/Convert/Traverse.hs index d1f24f2..8c22110 100644 --- a/src/Convert/Traverse.hs +++ b/src/Convert/Traverse.hs @@ -11,6 +11,7 @@ module Convert.Traverse , unmonad , collectify , mapBothM +, breakGenerate , traverseDescriptionsM , traverseDescriptions , collectDescriptionsM @@ -132,21 +133,21 @@ traverseDescriptions = map collectDescriptionsM :: Monad m => CollectorM m Description -> CollectorM m AST collectDescriptionsM = mapM_ -breakGenerate :: ModuleItem -> [ModuleItem] -breakGenerate (Generate genItems) = - if all isGenModuleItem genItems - then map (\(GenModuleItem item) -> item) genItems - else [Generate genItems] - where - isGenModuleItem :: GenItem -> Bool - isGenModuleItem (GenModuleItem _) = True - isGenModuleItem _ = False -breakGenerate other = [other] +breakGenerate :: ModuleItem -> [ModuleItem] -> [ModuleItem] +breakGenerate (Generate genItems) items = + foldr breakGenerateStep items genItems +breakGenerate item items = item : items + +breakGenerateStep :: GenItem -> [ModuleItem] -> [ModuleItem] +breakGenerateStep (GenModuleItem item) items = item : items +breakGenerateStep genItem (Generate genItems : items) = + Generate (genItem : genItems) : items +breakGenerateStep genItem items = Generate [genItem] : items traverseModuleItemsM :: Monad m => MapperM m ModuleItem -> MapperM m Description traverseModuleItemsM mapper (Part attrs extern kw lifetime name ports items) = do items' <- mapM (traverseNestedModuleItemsM mapper) items - let items'' = concatMap breakGenerate items' + let items'' = foldr breakGenerate [] items' return $ Part attrs extern kw lifetime name ports items'' where traverseModuleItemsM mapper (PackageItem packageItem) = do @@ -159,18 +160,18 @@ traverseModuleItemsM mapper (Package lifetime name items) = do let itemsWrapped = map MIPackageItem items itemsWrapped' <- mapM (traverseNestedModuleItemsM mapper) itemsWrapped let items' = map (\(MIPackageItem item) -> item) $ - concatMap breakGenerate itemsWrapped' + foldr breakGenerate [] itemsWrapped' return $ Package lifetime name items' traverseModuleItemsM mapper (Class lifetime name decls items) = do let declsWrapped = map (MIPackageItem . Decl) decls declsWrapped' <- mapM (traverseNestedModuleItemsM mapper) declsWrapped let decls' = map (\(MIPackageItem (Decl decl)) -> decl) $ - concatMap breakGenerate declsWrapped' + foldr breakGenerate [] declsWrapped' items' <- fmap concat $ mapM indirect items return $ Class lifetime name decls' items' where indirect (qualifier, item) = - fmap (map (unwrap qualifier) . breakGenerate) $ + fmap (map (unwrap qualifier) . flip breakGenerate []) $ traverseNestedModuleItemsM mapper (MIPackageItem item) unwrap qualifier = \(MIPackageItem item) -> (qualifier, item) diff --git a/test/core/paramtype.sv b/test/core/paramtype.sv index 892c319..84aad6a 100644 --- a/test/core/paramtype.sv +++ b/test/core/paramtype.sv @@ -126,3 +126,4 @@ module p_6; parameter W = 2; p #(logic [$signed(W):0], logic [$signed(W):0]) x() module p_7; parameter W = 2; p #(logic [square(W):0], logic [square(W):0]) x(); endmodule module p_8; p #(OtherT) x(); endmodule module p_9; p #(SomeT, OtherT) x(); endmodule +module p_A; if (1) begin : blk p #(SomeT, OtherT) x(); end endmodule diff --git a/test/core/paramtype.v b/test/core/paramtype.v index 14f138c..091ca2e 100644 --- a/test/core/paramtype.v +++ b/test/core/paramtype.v @@ -54,5 +54,7 @@ module top; $display("p 1 00000000000000000000000000000010 1"); $display("p 0000000000 00000000000000000000000000000001 10"); $display("p 0000000001 00000000000000000000000000000010 10"); + $display("p 0000000000 00000000000000000000000000000001 10"); + $display("p 0000000001 00000000000000000000000000000010 10"); end endmodule