From 13c84e4c7a22c3e30a0d694827e85befef0f3274 Mon Sep 17 00:00:00 2001 From: Zachary Snow Date: Mon, 31 May 2021 14:13:49 -0400 Subject: [PATCH] refactor parameter binding resolution - fix type/expr ambiguity for interface and class parameters - check for parameter kind mismatch up front - centralize key logic in ResolveBindings --- src/Convert/Package.hs | 11 ++- src/Convert/ParamType.hs | 40 +------- src/Convert/ResolveBindings.hs | 104 +++++++++++++++----- src/Language/SystemVerilog/AST.hs | 13 --- test/basic/ambiguous_tore.sv | 74 ++++++++++++++ test/basic/ambiguous_tore.v | 50 ++++++++++ test/error/interface_param_mismatch_expr.sv | 6 +- test/error/interface_param_mismatch_type.sv | 6 +- test/error/module_param_mismatch_expr.sv | 8 ++ test/error/module_param_mismatch_type.sv | 8 ++ 10 files changed, 239 insertions(+), 81 deletions(-) create mode 100644 test/basic/ambiguous_tore.sv create mode 100644 test/basic/ambiguous_tore.v create mode 100644 test/error/module_param_mismatch_expr.sv create mode 100644 test/error/module_param_mismatch_type.sv diff --git a/src/Convert/Package.hs b/src/Convert/Package.hs index 5b29005..a613011 100644 --- a/src/Convert/Package.hs +++ b/src/Convert/Package.hs @@ -30,6 +30,7 @@ import Data.Maybe (mapMaybe) import qualified Data.Map.Strict as Map import qualified Data.Set as Set +import Convert.ResolveBindings (exprToType, resolveBindings) import Convert.Scoper import Convert.Traverse import Language.SystemVerilog.AST @@ -504,11 +505,13 @@ resolveCSIdent className paramBindings scopeKeys itemName = do Decl $ ParamType Parameter x $ case lookup x' bindings of Just (Left t') -> t' - Just (Right (Ident y)) -> Alias y [] Just (Right e') -> - error $ "cannot override type parameter " ++ show x' - ++ " in class " ++ show className - ++ " with expression " ++ show e' + case exprToType e' of + Just t' -> t' + Nothing -> + error $ "cannot override type parameter " + ++ show x' ++ " in class " ++ show className + ++ " with expression " ++ show e' Nothing -> if t == UnknownType then error $ "required type parameter " ++ show x' diff --git a/src/Convert/ParamType.hs b/src/Convert/ParamType.hs index 1be75f4..7e0fe3d 100644 --- a/src/Convert/ParamType.hs +++ b/src/Convert/ParamType.hs @@ -13,7 +13,6 @@ import Data.Maybe (isJust, isNothing, fromJust) import qualified Data.Map.Strict as Map import qualified Data.Set as Set -import Convert.ExprUtils import Convert.Traverse import Language.SystemVerilog.AST @@ -241,25 +240,6 @@ isDefaultName m = defaultTag :: Identifier defaultTag = "_sv2v_default" --- attempt to convert an expression to syntactically equivalent type -exprToType :: Expr -> Maybe Type -exprToType (Ident x) = Just $ Alias x [] -exprToType (PSIdent y x) = Just $ PSAlias y x [] -exprToType (CSIdent y p x) = Just $ CSAlias y p x [] -exprToType (Range e NonIndexed r) = - case exprToType e of - Nothing -> Nothing - Just t -> Just $ tf (rs ++ [r]) - where (tf, rs) = typeRanges t -exprToType (Bit e i) = - case exprToType e of - Nothing -> Nothing - Just t -> Just $ tf (rs ++ [r]) - where - (tf, rs) = typeRanges t - r = (simplify $ BinOp Sub i (RawNum 1), RawNum 0) -exprToType _ = Nothing - -- checks where a type is sufficiently resolved to be substituted isSimpleType :: Type -> Bool isSimpleType typ = @@ -352,21 +332,11 @@ convertModuleItemM info (orig @ (Instance m bindings x r p)) = Map.toList resolvedTypesWithDecls resolveType :: Identifier -> Maybe Type -> (Type, (IdentSet, [Decl])) resolveType paramName defaultType = - case (Map.lookup paramName bindingsMap, defaultType) of - (Nothing, Just t) -> (t, (Set.empty, [])) - (Nothing, Nothing) -> - error $ "instantiation " ++ show orig ++ - " is missing a type parameter: " ++ paramName - (Just (Left t), _) -> prepareTypeExprs x paramName t - (Just (Right e), _) -> - -- Some types are parsed as expressions because of the - -- ambiguities of defined type names. - case exprToType e of - Just t -> prepareTypeExprs x paramName t - Nothing -> - error $ "instantiation " ++ show orig - ++ " has expr " ++ show e - ++ " for type param: " ++ paramName + case Map.lookup paramName bindingsMap of + Nothing -> (t, (Set.empty, [])) + where Just t = defaultType + Just b -> prepareTypeExprs x paramName t + where Left t = b -- leave only the normal expression params behind isParamType = flip Map.member maybeTypeMap diff --git a/src/Convert/ResolveBindings.hs b/src/Convert/ResolveBindings.hs index 6498640..b95b3b8 100644 --- a/src/Convert/ResolveBindings.hs +++ b/src/Convert/ResolveBindings.hs @@ -1,61 +1,70 @@ +{-# LANGUAGE TupleSections #-} {- sv2v - Author: Zachary Snow - - Conversion for `.*` and unnamed bindings - - While positional bindings need not be converted, resolving them here - - simplifies downstream conversions. + - simplifies downstream conversions. This conversion is also responsible for + - performing some basic validation and resolving the ambiguity between types + - and expressions in parameter binding contexts. -} -module Convert.ResolveBindings (convert) where +module Convert.ResolveBindings + ( convert + , exprToType + , resolveBindings + ) where import Control.Monad.Writer.Strict import qualified Data.Map.Strict as Map +import Convert.ExprUtils (simplify) import Convert.Traverse import Language.SystemVerilog.AST -type Ports = Map.Map Identifier ([Identifier], [Identifier]) +type Parts = Map.Map Identifier ([(Identifier, Bool)], [Identifier]) convert :: [AST] -> [AST] convert = traverseFiles - (collectDescriptionsM collectPortsM) + (collectDescriptionsM collectPartsM) (traverseDescriptions . traverseModuleItems . mapInstance) -collectPortsM :: Description -> Writer Ports () -collectPortsM (Part _ _ _ _ name ports items) = +collectPartsM :: Description -> Writer Parts () +collectPartsM (Part _ _ _ _ name ports items) = tell $ Map.singleton name (params, ports) - where params = parameterNames items -collectPortsM _ = return () + where params = parameterInfos items +collectPartsM _ = return () --- given a list of module items, produces the parameter names in order -parameterNames :: [ModuleItem] -> [Identifier] -parameterNames = +-- given a list of module items, produces the parameters in order +parameterInfos :: [ModuleItem] -> [(Identifier, Bool)] +parameterInfos = execWriter . mapM (collectNestedModuleItemsM $ collectDeclsM collectDeclM) where - collectDeclM :: Decl -> Writer [Identifier] () - collectDeclM (Param Parameter _ x _) = tell [x] - collectDeclM (ParamType Parameter x _) = tell [x] + collectDeclM :: Decl -> Writer [(Identifier, Bool)] () + collectDeclM (Param Parameter _ x _) = tell [(x, False)] + collectDeclM (ParamType Parameter x _) = tell [(x, True)] collectDeclM _ = return () -mapInstance :: Ports -> ModuleItem -> ModuleItem -mapInstance modulePorts (Instance m paramBindings x rs portBindings) = +mapInstance :: Parts -> ModuleItem -> ModuleItem +mapInstance parts (Instance m paramBindings x rs portBindings) = -- if we can't find it, just skip :( - if maybeModuleInfo == Nothing + if maybePartInfo == Nothing then Instance m paramBindings x rs portBindings else Instance m paramBindings' x rs portBindings' where - maybeModuleInfo = Map.lookup m modulePorts - Just (paramNames, portNames) = maybeModuleInfo + maybePartInfo = Map.lookup m parts + Just (paramInfos, portNames) = maybePartInfo + paramNames = map fst paramInfos msg :: String -> String msg = flip (++) $ " in instance " ++ show x ++ " of " ++ show m - paramBindings' = resolveBindings (msg "parameter overrides") paramNames - paramBindings - portBindings' = resolveBindings (msg "port connections") portNames - $ concatMap expandStar portBindings + paramBindings' = map checkParam $ + resolveBindings (msg "parameter overrides") paramNames paramBindings + portBindings' = resolveBindings (msg "port connections") portNames $ + concatMap expandStar portBindings expandStar :: PortBinding -> [PortBinding] expandStar ("*", Nil) = @@ -64,4 +73,53 @@ mapInstance modulePorts (Instance m paramBindings x rs portBindings) = where alreadyBound = map fst portBindings expandStar other = [other] + -- ensures parameter and binding kinds (type vs. expr) match + checkParam :: ParamBinding -> ParamBinding + checkParam (paramName, Right e) = + (paramName, ) $ + if isType + then case exprToType e of + Nothing -> kindMismatch paramName "a type" "expression" e + Just t -> Left t + else Right e + where Just isType = lookup paramName paramInfos + checkParam (paramName, Left t) = + if isType + then (paramName, Left t) + else kindMismatch paramName "an expression" "type" t + where Just isType = lookup paramName paramInfos + + kindMismatch :: Show k => Identifier -> String -> String -> k -> a + kindMismatch paramName expected actual value = + error $ msg ("parameter " ++ show paramName) + ++ " expects " ++ expected + ++ ", but was given " ++ actual + ++ ' ' : show value + mapInstance _ other = other + +-- attempt to convert an expression to syntactically equivalent type +exprToType :: Expr -> Maybe Type +exprToType (Ident x) = Just $ Alias x [] +exprToType (PSIdent y x) = Just $ PSAlias y x [] +exprToType (CSIdent y p x) = Just $ CSAlias y p x [] +exprToType (Range e NonIndexed r) = do + (tf, rs) <- fmap typeRanges $ exprToType e + Just $ tf (rs ++ [r]) +exprToType (Bit e i) = do + (tf, rs) <- fmap typeRanges $ exprToType e + let r = (simplify $ BinOp Sub i (RawNum 1), RawNum 0) + Just $ tf (rs ++ [r]) +exprToType _ = Nothing + +type Binding t = (Identifier, t) +-- give a set of bindings explicit names +resolveBindings :: String -> [Identifier] -> [Binding t] -> [Binding t] +resolveBindings _ _ [] = [] +resolveBindings location available bindings = + if length available < length bindings then + error $ "too many bindings specified for " ++ location + else if null $ fst $ head bindings then + zip available $ map snd bindings + else + bindings diff --git a/src/Language/SystemVerilog/AST.hs b/src/Language/SystemVerilog/AST.hs index fdef780..14dc2bb 100644 --- a/src/Language/SystemVerilog/AST.hs +++ b/src/Language/SystemVerilog/AST.hs @@ -29,7 +29,6 @@ module Language.SystemVerilog.AST , exprToLHS , lhsToExpr , shortHash - , resolveBindings ) where import Text.Printf (printf) @@ -85,15 +84,3 @@ shortHash :: (Show a) => a -> String shortHash x = take 5 $ printf "%05X" val where val = hash $ show x - -type Binding t = (Identifier, t) --- give a set of bindings explicit names -resolveBindings :: String -> [Identifier] -> [Binding t] -> [Binding t] -resolveBindings _ _ [] = [] -resolveBindings location available bindings = - if length available < length bindings then - error $ "too many bindings specified for " ++ location - else if null $ fst $ head bindings then - zip available $ map snd bindings - else - bindings diff --git a/test/basic/ambiguous_tore.sv b/test/basic/ambiguous_tore.sv new file mode 100644 index 0000000..f9599b8 --- /dev/null +++ b/test/basic/ambiguous_tore.sv @@ -0,0 +1,74 @@ +class C #( + parameter PARAM_E = 1, + parameter type PARAM_T = logic +); + localparam E = PARAM_E * $bits(PARAM_T); + localparam type T = PARAM_T [PARAM_E - 1:0]; +endclass + +package P; + localparam E = 2; + localparam type T = logic [E - 1:0]; +endpackage + +module M #( + parameter DELAY, + parameter PREFIX, + parameter SUFFIX, + parameter E, + parameter type T +); + initial #DELAY $display("M: %sE%s=%0d $bits(%sT%s)=%0d", + PREFIX, SUFFIX, E, PREFIX, SUFFIX, $bits(T)); +endmodule + +interface I #( + parameter DELAY, + parameter PREFIX, + parameter SUFFIX, + parameter E, + parameter type T +); + initial #DELAY $display("I: %sE%s=%0d $bits(%sT%s)=%0d", + PREFIX, SUFFIX, E, PREFIX, SUFFIX, $bits(T)); +endinterface + +module top; + parameter FLAG = 1; + + localparam E = 3; + localparam type T = logic [E * 2 - 1:0]; + + `define TEST(D, prefix) \ + if (FLAG) begin \ + localparam the_expr = prefix``E; \ + localparam type the_type = prefix``T; \ + initial begin \ + #(D * 10); \ + $display(`"prefix``E = %0d %0d`", the_expr, prefix``E); \ + $display(`"$bits(prefix``E) = %0d %0d`", $bits(the_expr), $bits(prefix``E)); \ + $display(`"$bits(prefix``T) = %0d %0d`", $bits(the_type), $bits(prefix``T)); \ + $display(`"$left(prefix``E) = %0d`", $left(prefix``E)); \ + $display(`"$left(prefix``T) = %0d`", $left(prefix``T)); \ + $display(`"prefix``E'('z) = %b`", prefix``E'('z)); \ + $display(`"prefix``T'('z) = %b`", prefix``T'('z)); \ + end \ + I #(D*10+1, `"prefix`", "", prefix``E, prefix``T) i1(); \ + M #(D*10+2, `"prefix`", "", prefix``E, prefix``T) m1(); \ + I #(D*10+3, `"prefix`", "[6]", prefix``E[6], prefix``T[6]) i2(); \ + M #(D*10+4, `"prefix`", "[6]", prefix``E[6], prefix``T[6]) m2(); \ + I #(D*10+5, `"prefix`", "[6:0]", prefix``E[6:0], prefix``T[6:0]) i3(); \ + M #(D*10+6, `"prefix`", "[6:0]", prefix``E[6:0], prefix``T[6:0]) m3(); \ + end + + `TEST(0, ) + `TEST(1, P::) + + `TEST(2, C#()::) + `TEST(3, C#(E, T)::) + `TEST(4, C#(P::E, P::T)::) + `TEST(5, C#(C#(E, T)::E, C#(E, T)::T)::) + `TEST(6, C#(E[1], T[2])::) + `TEST(7, C#(E[2:0], T[2:0])::) + +endmodule diff --git a/test/basic/ambiguous_tore.v b/test/basic/ambiguous_tore.v new file mode 100644 index 0000000..f993760 --- /dev/null +++ b/test/basic/ambiguous_tore.v @@ -0,0 +1,50 @@ +module MI; + parameter KIND = ""; + parameter DELAY = 0; + parameter PREFIX = ""; + parameter SUFFIX = ""; + parameter E = 1; + parameter T = 1; + initial #DELAY $display("%s: %sE%s=%0d $bits(%sT%s)=%0d", + KIND, PREFIX, SUFFIX, E, PREFIX, SUFFIX, T); +endmodule + +module top; + parameter FLAG = 1; + + `define TEST(D, prefix, eval, tval, leftval) \ + if (FLAG) begin \ + localparam [eval:1] ez = 1'sbz; \ + localparam [tval:1] tz = 1'sbz; \ + initial begin \ + #(D * 10); \ + $display(`"prefix``E = %0d %0d`", eval, eval); \ + $display(`"$bits(prefix``E) = %0d %0d`", 32, 32); \ + $display(`"$bits(prefix``T) = %0d %0d`", tval, tval); \ + $display(`"$left(prefix``E) = %0d`", 31); \ + $display(`"$left(prefix``T) = %0d`", leftval); \ + $display(`"prefix``E'('z) = %b`", ez); \ + $display(`"prefix``T'('z) = %b`", tz); \ + end \ + MI #("I", D*10+1, `"prefix`", "", eval, tval) i1(); \ + MI #("M", D*10+2, `"prefix`", "", eval, tval) m1(); \ + MI #("I", D*10+3, `"prefix`", "[6]", (eval >> 6) & 1'b1, tval*6) i2(); \ + MI #("M", D*10+4, `"prefix`", "[6]", (eval >> 6) & 1'b1, tval*6) m2(); \ + MI #("I", D*10+5, `"prefix`", "[6:0]", eval & 7'h7f, tval * 7) i3(); \ + MI #("M", D*10+6, `"prefix`", "[6:0]", eval & 7'h7f, tval * 7) m3(); \ + end + + `define TEST_C(D, prefix, efac, tfac) \ + `TEST(D, prefix, (efac)*(tfac), (tfac)*(efac), (efac)-1) + + `TEST(0, , 3, 6, 5) + `TEST(1, P::, 2, 2, 1) + + `TEST_C(2, C#()::, 1, 1) + `TEST_C(3, C#(E, T)::, 3, 6) + `TEST_C(4, C#(P::E, P::T)::, 2, 2) + `TEST_C(5, C#(C#(E, T)::E, C#(E, T)::T)::, 18, 18) + `TEST_C(6, C#(E[1], T[2])::, 1, 12) + `TEST_C(7, C#(E[2:0], T[2:0])::, 3, 18) + +endmodule diff --git a/test/error/interface_param_mismatch_expr.sv b/test/error/interface_param_mismatch_expr.sv index be11ab9..663b1fa 100644 --- a/test/error/interface_param_mismatch_expr.sv +++ b/test/error/interface_param_mismatch_expr.sv @@ -1,8 +1,8 @@ +// pattern: parameter "P" in instance "intf" of "Interface" expects an expression, but was given type logic interface Interface; - parameter T = 0; - logic [T-1:0] x; + parameter P = 0; + logic [P-1:0] x; endinterface - module top; Interface #(logic) intf(); endmodule diff --git a/test/error/interface_param_mismatch_type.sv b/test/error/interface_param_mismatch_type.sv index 7954d0a..07da7f2 100644 --- a/test/error/interface_param_mismatch_type.sv +++ b/test/error/interface_param_mismatch_type.sv @@ -1,8 +1,8 @@ +// pattern: parameter "P" in instance "intf" of "Interface" expects a type, but was given expression 1 interface Interface; - parameter type T; - T x; + parameter type P; + P x; endinterface - module top; Interface #(1) intf(); endmodule diff --git a/test/error/module_param_mismatch_expr.sv b/test/error/module_param_mismatch_expr.sv new file mode 100644 index 0000000..2f1c0b4 --- /dev/null +++ b/test/error/module_param_mismatch_expr.sv @@ -0,0 +1,8 @@ +// pattern: parameter "P" in instance "mod" of "Module" expects an expression, but was given type logic +module Module; + parameter P = 0; + logic [P-1:0] x; +endmodule +module top; + Module #(logic) mod(); +endmodule diff --git a/test/error/module_param_mismatch_type.sv b/test/error/module_param_mismatch_type.sv new file mode 100644 index 0000000..4866c59 --- /dev/null +++ b/test/error/module_param_mismatch_type.sv @@ -0,0 +1,8 @@ +// pattern: parameter "P" in instance "mod" of "Module" expects a type, but was given expression 1 +module Module; + parameter type P; + P x; +endmodule +module top; + Module #(1) mod(); +endmodule