Skip to content

Commit

Permalink
special case typenames when resolving pattern identifiers
Browse files Browse the repository at this point in the history
  • Loading branch information
zachjs committed Nov 4, 2023
1 parent d5b9c1d commit a4639fa
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 5 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
* Fixed an issue that prevented parsing tasks and functions with `inout` ports
* Fixed conflicting genvar names when inlining interfaces and modules that use
them; all genvars are now given a design-wide unique name
* Fixed non-typenames (e.g., from packages or subsequent declarations)
improperly shadowing the names of `struct` pattern fields
* Fixed failure to resolve typenames suffixed with dimensions in contexts
permitting both types and expressions, e.g., `$bits(T[W-1:0])`
* Fixed errant constant folding of shadowed non-trivial localparams
Expand Down
49 changes: 44 additions & 5 deletions src/Convert/Package.hs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ module Convert.Package

import Control.Monad.State.Strict
import Control.Monad.Writer.Strict
import Data.Functor ((<&>))
import Data.List (insert, intercalate, isPrefixOf)
import Data.Maybe (mapMaybe)
import qualified Data.Map.Strict as Map
Expand Down Expand Up @@ -261,6 +262,9 @@ explicitImport pkg ident = do
++ " conflicts with prior import of "
++ otherPkg ++ "::" ++ ident

pattern PatternTag :: Char
pattern PatternTag = '*'

-- main logic responsible for translating packages, resolving imports and
-- exports, and rewriting identifiers referring to package declarations
processItems :: Identifier -> Identifier -> [ModuleItem]
Expand Down Expand Up @@ -366,7 +370,10 @@ processItems topName packageName moduleItems = do
Variable d t x a e -> declHelp x $ \x' -> Variable d t x' a e
Net d n s t x a e -> declHelp x $ \x' -> Net d n s t x' a e
Param p t x e -> declHelp x $ \x' -> Param p t x' e
ParamType p x t -> declHelp x $ \x' -> ParamType p x' t
ParamType p x t ->
-- make this typename available for use in patterns
prefixIdent (PatternTag : x) >>
declHelp x (\x' -> ParamType p x' t)
CommentDecl c -> return $ CommentDecl c
where declHelp x f = prefixIdent x >>= return . f

Expand Down Expand Up @@ -409,10 +416,25 @@ processItems topName packageName moduleItems = do
x' <- resolvePSIdent' p x
return $ Ident x'
traverseExprM (Ident x) = resolveIdent x >>= return . Ident
traverseExprM (Pattern items) = do
keys' <- mapM keyMapper keys
vals' <- mapM traverseExprM vals
return $ Pattern $ zip keys' vals'
where (keys, vals) = unzip items
traverseExprM other =
traverseSinglyNestedExprsM traverseExprM other
>>= traverseExprTypesM traverseTypeM

-- equivalent to the special case in traverseExprIdentsM
keyMapper (Left t) =
traverseTypeM t <&> Left
keyMapper (Right (Ident x)) = do
tagged' <- traverseExprM $ Ident $ PatternTag : x
let Ident x' = tagged'
return $ Right $ Ident $ filter (/= PatternTag) x'
keyMapper (Right e) =
traverseExprM e <&> Right

traverseLHSM :: LHS -> Scope LHS
traverseLHSM (LHSIdent x) = resolveIdent x >>= return . LHSIdent
traverseLHSM other = traverseSinglyNestedLHSsM traverseLHSM other
Expand Down Expand Up @@ -783,11 +805,28 @@ traverseIdentsM identMapper = traverseNodesM

-- visits all identifiers in an expression
traverseExprIdentsM :: Monad m => MapperM m Identifier -> MapperM m Expr
traverseExprIdentsM identMapper = fullMapper
traverseExprIdentsM identMapper = exprMapper
where
fullMapper = exprMapper >=> traverseSinglyNestedExprsM fullMapper
exprMapper (Ident x) = identMapper x >>= return . Ident
exprMapper other = return other
exprMapper (Pattern items) = do
keys' <- mapM keyMapper keys
vals' <- mapM exprMapper vals
return $ Pattern $ zip keys' vals'
where (keys, vals) = unzip items
exprMapper other =
traverseExprTypesM (traverseTypeIdentsM identMapper) other >>=
traverseSinglyNestedExprsM exprMapper

-- Don't reorder or inject a pattern key unless it refers to a known
-- typename. TODO: This prevents referring to expressions in packages in
-- pattern keys, though this probably isn't common.
keyMapper (Left t) =
traverseTypeIdentsM identMapper t <&> Left
keyMapper (Right (Ident x)) =
identMapper (PatternTag : x)
<&> filter (/= PatternTag) <&> Ident <&> Right
keyMapper (Right e) =
exprMapper e <&> Right

-- visits all identifiers in a type
traverseTypeIdentsM :: Monad m => MapperM m Identifier -> MapperM m Type
Expand Down Expand Up @@ -830,7 +869,7 @@ piNames (Task _ ident _ _) = [ident]
piNames (Decl (Variable _ _ ident _ _)) = [ident]
piNames (Decl (Net _ _ _ _ ident _ _)) = [ident]
piNames (Decl (Param _ _ ident _)) = [ident]
piNames (Decl (ParamType _ ident _)) = [ident]
piNames (Decl (ParamType _ ident _)) = [ident, PatternTag : ident]
piNames (Decl (CommentDecl _)) = []
piNames item@DPIImport{} = [show item]
piNames item@DPIExport{} = [show item]
Expand Down
30 changes: 30 additions & 0 deletions test/core/pattern_resolve.sv
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package P;
function automatic integer F;
F = '1;
endfunction
typedef byte G;
endpackage

module top;
typedef struct packed {
integer unsigned X, Y, Z;
} T;
parameter T W = '{X: 3, Y: 5, Z: 7};
localparam Z = W.X;
localparam Y = W.Z;
initial $display(W, W.X, W.Y, W.Z, Z, Y);

// There is disagreement among commercial simulators on whether or not type
// names can shadow field names. sv2v allows this shadowing.
import P::*;
typedef struct packed {
byte E;
shortint F;
integer G;
} U;
U a = '{F: 1, G: 2, default: 3};
U b = '{E: 1, G: 2, default: 3};
U c = '{F: F(), default: 4};
`define DUMP(v) initial $display("%b %b %b", v.E, v.F, v.G);
`DUMP(a) `DUMP(b) `DUMP(c)
endmodule
15 changes: 15 additions & 0 deletions test/core/pattern_resolve.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module top;
parameter [95:0] W = {32'd3, 32'd5, 32'd7};
localparam Z = W[95:64];
localparam Y = W[31:0];
initial $display(W, W[95:64], W[63:32], W[31:0], Z, Y);

reg [55:0] a, b, c;
initial begin
a = {8'd2, 16'd1, 32'd3};
b = {8'd1, 16'd3, 32'd3};
c = {8'd4, 16'hFFFF, 32'd4};
`define DUMP(v) $display("%b %b %b", v[55:48], v[47:32], v[31:0]);
`DUMP(a) `DUMP(b) `DUMP(c)
end
endmodule

0 comments on commit a4639fa

Please sign in to comment.