Fix generate operators not short circuiting, bug413. Merge from Jeremy Bennett.

Changes

@@ -3,12 +3,9 @@ Revision history for Verilator
 The contributors that suggested a given feature are shown in [].  [by ...]
 indicates the contributor was also the author of the fix; Thanks!
 
-* Verilator 3.900 devel
+* Verilator 3.840 devel
-
-New features:
-
-Bug fixes:
 
+***   Fix generate operators not short circuiting, bug413.  [by Jeremy Bennett]
 
 * Verilator 3.833 2012/04/15
 

src/V3Const.cpp

@@ -107,6 +107,7 @@ private:
     bool	m_doNConst;	// Enable non-constant-child simplifications
     bool	m_doShort;	// Remove expressions that short circuit
     bool	m_doV;		// Verilog, not C++ conversion
+    bool	m_doGenerate;	// Postpone width checking inside generate
     AstNodeModule*	m_modp;		// Current module
     AstNode*	m_scopep;	// Current scope
 
@@ -368,6 +369,7 @@ private:
     // Extraction checks
     bool warnSelect(AstSel* nodep) {
 	AstNode* basefromp = AstArraySel::baseFromp(nodep);
+	if (m_doGenerate) V3Width::widthParamsEdit(nodep); // Never checked yet
 	if (AstNodeVarRef* varrefp = basefromp->castNodeVarRef()) {
 	    AstVar* varp = varrefp->varp();
 	    if (!varp->dtypep()) varp->v3fatalSrc("Data type lost");
@@ -528,6 +530,20 @@ private:
 	nodep->replaceWith(childp);
 	nodep->deleteTree(); nodep=NULL;
     }
+
+    //! Replace a ternary node with its RHS after iterating
+    //! Used with short-circuting, where the RHS has not yet been iterated.
+    void replaceWIteratedRhs(AstNodeTriop* nodep) {
+	if (AstNode *rhsp = nodep->rhsp()) rhsp->iterateAndNext(*this);
+	replaceWChild(nodep, nodep->rhsp());	// May have changed
+    }
+
+    //! Replace a ternary node with its THS after iterating
+    //! Used with short-circuting, where the THS has not yet been iterated.
+    void replaceWIteratedThs(AstNodeTriop* nodep) {
+	if (AstNode *thsp = nodep->thsp()) thsp->iterateAndNext(*this);
+	replaceWChild(nodep, nodep->thsp());	// May have changed
+    }
     void replaceWLhs(AstNodeUniop* nodep) {
 	// Keep LHS, remove RHS
 	replaceWChild(nodep, nodep->lhsp());
@@ -1681,6 +1697,8 @@ private:
     //    v--- *1* These ops are always first, as we warn before replacing
     //    v--- *V* This op is a verilog op, only in m_doV mode
     //    v--- *C* This op works on all constant children, allowed in m_doConst mode
+    //    v--- *S* This op specifies a type should use short-circuting of its lhs op
+
     TREEOP1("AstSel{warnSelect(nodep)}",	"NEVER");
     // Generic constants on both side.  Do this first to avoid other replacements
     TREEOPC("AstNodeBiop {$lhsp.castConst, $rhsp.castConst}",  "replaceConst(nodep)");
@@ -1688,7 +1706,11 @@ private:
     // Zero on one side or the other
     TREEOP ("AstAdd   {$lhsp.isZero, $rhsp}",	"replaceWRhs(nodep)");
     TREEOP ("AstAnd   {$lhsp.isZero, $rhsp, isTPure($rhsp)}",	"replaceZero(nodep)");  // Can't use replaceZeroChkPure as we make this pattern in ChkPure
+    // This visit function here must allow for short-circuiting.
+    TREEOPS("AstLogAnd   {$lhsp.isZero}",	"replaceZero(nodep)");
     TREEOP ("AstLogAnd{$lhsp.isZero, $rhsp}",	"replaceZero(nodep)");
+    // This visit function here must allow for short-circuiting.
+    TREEOPS("AstLogOr   {$lhsp.isOne}",		"replaceNum(nodep, 1)");
     TREEOP ("AstLogOr {$lhsp.isZero, $rhsp}",	"replaceWRhs(nodep)");
     TREEOP ("AstDiv   {$lhsp.isZero, $rhsp}",	"replaceZeroChkPure(nodep,$rhsp)");
     TREEOP ("AstDivS  {$lhsp.isZero, $rhsp}",	"replaceZeroChkPure(nodep,$rhsp)");
@@ -1742,6 +1764,9 @@ private:
     TREEOPC("AstNodeCond{$condp.isZero,       $expr1p.castConst, $expr2p.castConst}", "replaceWChild(nodep,$expr2p)");
     TREEOPC("AstNodeCond{$condp.isNeqZero,    $expr1p.castConst, $expr2p.castConst}", "replaceWChild(nodep,$expr1p)");
     TREEOP ("AstNodeCond{$condp, operandsSame($expr1p,,$expr2p)}","replaceWChild(nodep,$expr1p)");
+    // This visit function here must allow for short-circuiting.
+    TREEOPS("AstCond {$lhsp.isZero}",		"replaceWIteratedThs(nodep)");
+    TREEOPS("AstCond {$lhsp.isNeqZero}",	"replaceWIteratedRhs(nodep)");
     TREEOP ("AstCond{$condp->castNot(),       $expr1p, $expr2p}", "AstCond{$condp->op1p(), $expr2p, $expr1p}");
     TREEOP ("AstNodeCond{$condp.width1, $expr1p.width1,   $expr1p.isAllOnes, $expr2p}", "AstLogOr {$condp, $expr2p}");  // a?1:b == a||b
     TREEOP ("AstNodeCond{$condp.width1, $expr1p.width1,   $expr1p,    $expr2p.isZero}", "AstLogAnd{$condp, $expr1p}");  // a?b:0 == a&&b
@@ -1915,6 +1940,8 @@ private:
     TREEOPV("AstSel{$fromp.castXnor,$lhsp.castConst}",	"replaceSelIntoUniop(nodep)");
     // Conversions
     TREEOPV("AstRedXnor{$lhsp}",		"AstNot{AstRedXor{$lhsp}}");  // Just eliminate XNOR's
+    // This visit function here must allow for short-circuiting.
+    TREEOPS("AstLogIf {$lhsp.isZero}",		"replaceNum(nodep, 1)");
     TREEOPV("AstLogIf {$lhsp, $rhsp}",		"AstLogOr{AstLogNot{$lhsp},$rhsp}");
     TREEOPV("AstLogIff{$lhsp, $rhsp}",		"AstLogNot{AstXor{$lhsp,$rhsp}}");
     // Strings
@@ -1948,6 +1975,7 @@ public:
     // Processing Mode Enum
     enum ProcMode {
 	PROC_PARAMS,
+	PROC_GENERATE,
 	PROC_LIVE,
 	PROC_V_WARN,
 	PROC_V_NOWARN,
@@ -1963,6 +1991,7 @@ public:
 	m_doNConst = false;
 	m_doShort = true;	// Presently always done
 	m_doV = false;
+	m_doGenerate = false;	// Inside generate conditionals
 	m_warn = false;
 	m_wremove = true;  // Overridden in visitors
 	m_modp = NULL;
@@ -1970,6 +1999,7 @@ public:
 	//
 	switch (pmode) {
 	case PROC_PARAMS:	m_doV = true;  m_doNConst = true; m_params = true; m_required = true; break;
+	case PROC_GENERATE:	m_doV = true;  m_doNConst = true; m_params = true; m_required = true; m_doGenerate = true; break;
 	case PROC_LIVE:		break;
 	case PROC_V_WARN:	m_doV = true;  m_doNConst = true; m_warn = true; break;
 	case PROC_V_NOWARN:	m_doV = true;  m_doNConst = true; break;
@@ -1988,12 +2018,46 @@ public:
 //######################################################################
 // Const class functions
 
+//! Force this cell node's parameter list to become a constant
+//! @return  Pointer to the edited node.
 AstNode* V3Const::constifyParamsEdit(AstNode* nodep) {
     //if (debug()>0) nodep->dumpTree(cout,"  forceConPRE : ");
-    // Resize even if the node already has a width, because burried in the treee we may
+    // Resize even if the node already has a width, because buried in the tree
-    // have a node we just created with signing, etc, that isn't sized yet.
+    // we may have a node we just created with signing, etc, that isn't sized yet.
-    nodep = V3Width::widthParamsEdit(nodep); // Make sure we've sized everything first
+
-    ConstVisitor visitor (ConstVisitor::PROC_PARAMS);
+    // Make sure we've sized everything first
+    nodep = V3Width::widthParamsEdit(nodep);
+    ConstVisitor visitor(ConstVisitor::PROC_PARAMS);
+    if (AstVar* varp=nodep->castVar()) {
+	// If a var wants to be constified, it's really a param, and
+	// we want the value to be constant.  We aren't passed just the
+	// init value because we need widthing above to handle the var's type.
+	if (varp->valuep()) visitor.mainAcceptEdit(varp->valuep());
+    } else {
+	nodep = visitor.mainAcceptEdit(nodep);
+    }
+    // Because we do edits, nodep links may get trashed and core dump this.
+    //if (debug()>0) nodep->dumpTree(cout,"  forceConDONE: ");
+    return nodep;
+}
+
+//! Force this cell node's parameter list to become a constant inside generate.
+//! If we are inside a generated "if", "case" or "for", we don't want to
+//! trigger warnings when we deal with the width. It is possible that these
+//! are spurious, existing within sub-expressions that will not actually be
+//! generated. Since such occurrences, must be constant, in order to be
+//! someting a generate block can depend on, we can wait until later to do the
+//! width check.
+//! @return  Pointer to the edited node.
+AstNode* V3Const::constifyGenerateParamsEdit(AstNode* nodep) {
+    //if (debug()>0) nodep->dumpTree(cout,"  forceConPRE : ");
+    // Resize even if the node already has a width, because buried in the tree
+    // we may have a node we just created with signing, etc, that isn't sized
+    // yet.
+
+    // Make sure we've sized everything first
+    nodep = V3Width::widthGenerateParamsEdit(nodep);
+    ConstVisitor visitor(ConstVisitor::PROC_GENERATE);
     if (AstVar* varp=nodep->castVar()) {
 	// If a var wants to be constified, it's really a param, and
 	// we want the value to be constant.  We aren't passed just the

src/V3Const.h

@@ -31,9 +31,8 @@
 
 class V3Const {
 public:
-    // Force this cell node's parameter list to become a constant
-    // Return new node that may have replaced nodep
     static AstNode* constifyParamsEdit(AstNode* nodep);
+    static AstNode* constifyGenerateParamsEdit(AstNode* nodep);
     // Only do constant pushing, without removing dead logic
     static void constifyAllLive(AstNetlist* nodep);
     // Everything that's possible

src/V3Param.cpp

@@ -189,8 +189,12 @@ private:
     virtual void visit(AstGenIf* nodep, AstNUser*) {
 	UINFO(9,"  GENIF "<<nodep<<endl);
 	nodep->condp()->iterateAndNext(*this);
-	V3Width::widthParamsEdit(nodep);  // Param typed widthing will NOT recurse the body
+	// We suppress errors when widthing params since short-circuiting in
-	V3Const::constifyParamsEdit(nodep->condp());  // condp may change
+	// the conditional evaluation may mean these error can never occur. We
+	// then make sure that short-circuiting is used by constifyParamsEdit.
+	V3Width::widthGenerateParamsEdit(nodep);  // Param typed widthing will
+						  // NOT recurse the body.
+	V3Const::constifyGenerateParamsEdit(nodep->condp()); // condp may change
 	if (AstConst* constp = nodep->condp()->castConst()) {
 	    AstNode* keepp = (constp->isZero()
 			      ? nodep->elsesp()
@@ -207,6 +211,11 @@ private:
 	    nodep->condp()->v3error("Generate If condition must evaluate to constant");
 	}
     }
+
+    //! Parameter subsitution for generated for loops.
+    //! @todo Unlike generated IF, we don't have to worry about short-circuiting the conditional
+    //!       expression, since this is currently restricted to simple comparisons. If we ever do
+    //!       move to more generic constant expressions, such code will be neede here.
     virtual void visit(AstGenFor* nodep, AstNUser*) {
 	// We parse a very limited form of FOR, so we don't need to do a full
 	// simulation to unroll the loop
@@ -220,7 +229,8 @@ private:
 	AstNode* keepp = NULL;
 	nodep->exprp()->iterateAndNext(*this);
 	V3Case::caseLint(nodep);
-	V3Width::widthParamsEdit(nodep);  // Param typed widthing will NOT recurse the body
+	V3Width::widthParamsEdit(nodep);  // Param typed widthing will NOT recurse the body,
+					  // don't trigger errors yet.
 	V3Const::constifyParamsEdit(nodep->exprp());  // exprp may change
 	AstConst* exprp = nodep->exprp()->castConst();
 	// Constify

src/V3Width.cpp

@@ -110,6 +110,7 @@ private:
     AstRange*	m_cellRangep;	// Range for arrayed instantiations, NULL for normal instantiations
     AstNodeCase* m_casep;	// Current case statement CaseItem is under
     AstFunc*	m_funcp;	// Current function
+    bool	m_doGenerate;	// Do errors later inside generate statement
 
     // CLASSES
 #define ANYSIZE 0
@@ -436,15 +437,31 @@ private:
 		// See also warning in V3Const
 		// We need to check here, because the widthCheck may silently
 		// add another SEL which will lose the out-of-range check
-		nodep->v3error("Selection index out of range: "
+		//
- 			       <<nodep->msbConst()<<":"<<nodep->lsbConst()
+		// We don't want to trigger an error here if we are just
-			       <<" outside "<<frommsb<<":"<<fromlsb);
+		// evaluating type sizes for a generate block condition. We
-		UINFO(1,"    Related node: "<<nodep<<endl);
+		// should only trigger the error if the out-of-range access is
-		if (varrp) UINFO(1,"    Related var: "<<varrp->varp()<<endl);
+		// actually generated.
-		if (varrp) UINFO(1,"    Related dtype: "<<varrp->varp()->dtypep()<<endl);
+		if (m_doGenerate) {
+		    UINFO(5, "Selection index out of range inside generate."<<endl);
+		} else {
+		    nodep->v3error("Selection index out of range: "
+				   <<nodep->msbConst()<<":"<<nodep->lsbConst()
+				   <<" outside "<<frommsb<<":"<<fromlsb);
+		    UINFO(1,"    Related node: "<<nodep<<endl);
+		    if (varrp) UINFO(1,"    Related var: "<<varrp->varp()<<endl);
+		    if (varrp) UINFO(1,"    Related dtype: "<<varrp->varp()->dtypep()<<endl);
+		}
 	    }
 	    // iterate FINAL is two blocks above
-	    widthCheck(nodep,"Extract Range",nodep->lsbp(),selwidth,selwidth,true);
+	    //
+	    // If we have a width problem with GENERATE etc, this will reduce
+	    // it down and mask it, so we have no chance of finding a real
+	    // error in the future. So don't do this for them.
+	    if (!m_doGenerate) {
+		widthCheck(nodep,"Extract Range",nodep->lsbp(),selwidth,
+			   selwidth,true);
+	    }
 	}
     }
 
@@ -1961,11 +1978,14 @@ private:
 
 public:
     // CONSTUCTORS
-    WidthVisitor(bool paramsOnly) {
+    WidthVisitor(bool paramsOnly,   // [in] TRUE if we are considering parameters only.
+	         bool doGenerate) { // [in] TRUE if we are inside a generate statement and
+	//			    // don't wish to trigger errors
 	m_paramsOnly = paramsOnly;
 	m_cellRangep = NULL;
 	m_casep = NULL;
 	m_funcp = NULL;
+	m_doGenerate = doGenerate;
     }
     AstNode* mainAcceptEdit(AstNode* nodep) {
 	return nodep->acceptSubtreeReturnEdits(*this, WidthVP(ANYSIZE,0,BOTH).p());
@@ -1979,16 +1999,38 @@ public:
 void V3Width::width(AstNetlist* nodep) {
     UINFO(2,__FUNCTION__<<": "<<endl);
     // We should do it in bottom-up module order, but it works in any order.
-    WidthVisitor visitor (false);
+    WidthVisitor visitor (false, false);
     (void)visitor.mainAcceptEdit(nodep);
     WidthRemoveVisitor rvisitor;
     (void)rvisitor.mainAcceptEdit(nodep);
 }
 
-AstNode* V3Width::widthParamsEdit(AstNode* nodep) {
+//! Single node parameter propagation
+//! Smaller step... Only do a single node for parameter propagation
+AstNode* V3Width::widthParamsEdit (AstNode* nodep) {
     UINFO(4,__FUNCTION__<<": "<<endl);
     // We should do it in bottom-up module order, but it works in any order.
-    WidthVisitor visitor (true);
+    WidthVisitor visitor (true, false);
+    nodep = visitor.mainAcceptEdit(nodep);
+    WidthRemoveVisitor rvisitor;
+    nodep = rvisitor.mainAcceptEdit(nodep);
+    return nodep;
+}
+
+//! Single node parameter propagation for generate blocks.
+//! Smaller step... Only do a single node for parameter propagation
+//! If we are inside a generated "if", "case" or "for", we don't want to
+//! trigger warnings when we deal with the width. It is possible that
+//! these are spurious, existing within sub-expressions that will not
+//! actually be generated. Since such occurrences, must be constant, in
+//! order to be someting a generate block can depend on, we can wait until
+//! later to do the width check.
+//! @return  Pointer to the edited node.
+AstNode* V3Width::widthGenerateParamsEdit(
+    AstNode* nodep) { //!< [in] AST whose parameters widths are to be analysed.
+    UINFO(4,__FUNCTION__<<": "<<endl);
+    // We should do it in bottom-up module order, but it works in any order.
+    WidthVisitor visitor (true, true);
     nodep = visitor.mainAcceptEdit(nodep);
     WidthRemoveVisitor rvisitor;
     nodep = rvisitor.mainAcceptEdit(nodep);

src/V3Width.h

@@ -32,8 +32,8 @@
 class V3Width {
 public:
     static void width(AstNetlist* nodep);
-    // Smaller step... Only do a single node for parameter propagation
     static AstNode* widthParamsEdit(AstNode* nodep);
+    static AstNode* widthGenerateParamsEdit(AstNode* nodep);
     // Final step... Mark all widths as equal
     static void widthCommit(AstNetlist* nodep);
 

src/astgen

@@ -373,7 +373,8 @@ sub tree_line {
     $func =~ s!\s*//.*$!!;
     $func =~ s!\s*;\s*$!!;
 
-    if ($func =~ /TREEOP(1?)([VC]?)\s*\(\s*  \"([^\"]*)\"  \s*,\s* \"([^\"]*)\"  \s*\)/sx) {
+    # doflag "S" indicates an op specifying short-circuiting for a type.
+    if ($func =~ /TREEOP(1?)([VCS]?)\s*\(\s*  \"([^\"]*)\"  \s*,\s* \"([^\"]*)\"  \s*\)/sx) {
 	my $order = $1; my $doflag = $2; my $from = $3; my $to = $4;
 	#$self->print("// $from $to\n");
 	if (!$self->{did_out_tree}) {
@@ -393,6 +394,7 @@ sub tree_line {
 	if ($doflag eq '') { $mif = "m_doNConst"; }
 	elsif ($doflag eq 'V') { $mif = "m_doV"; }
 	elsif ($doflag eq 'C') { $mif = ""; }
+	elsif ($doflag eq 'S') { $mif = "m_doNConst"; } # Not just for m_doGenerate
 	else { die; }
 	$subnodes =~ s/,,/__ESCAPEDCOMMA__/g;
 	foreach my $subnode (split /\s*,\s*/, $subnodes) {
@@ -416,7 +418,9 @@ sub tree_line {
 	    match_if => $mif,
 	    exec_func => $exec_func,
 	    uinfo_level => ($to =~ /^!/ ? 0:7),
+	    short_circuit => ($doflag eq 'S'),
 	};
+
 	($typefunc->{uinfo} = $func) =~ s/[ \t\"\{\}]+/ /g;
 	push @{$self->{treeop}{$type}}, $typefunc;
     }
@@ -559,22 +563,47 @@ sub tree_base {
     $self->print ("    // Bottom class up, as more simple transforms are generally better\n");
     foreach my $type (sort (keys %::Classes)) {
 	my $base = $::Classes{$type};
+	my @out_for_type_sc;
 	my @out_for_type;
 	foreach my $base (::subclasses_of($type), $type) {
 	    foreach my $typefunc (@{$self->{treeop}{$base}}) {
 		my @lines = ("	if ($typefunc->{match_func}(nodep)) return;\n",);
-		if ($typefunc->{order}) {
+		if ($typefunc->{short_circuit}) {	# short-circuit match fn
-		    unshift @out_for_type, @lines;   # TREEOP1's go in front of others
+		    push @out_for_type_sc, @lines;
-		} else {
+		} else {				# Standard match fn
-		    push @out_for_type, @lines;
+		    if ($typefunc->{order}) {
+			unshift @out_for_type, @lines;   # TREEOP1's go in front of others
+		    } else {
+			push @out_for_type, @lines;
+		    }
 		}
 	    }
 	}
-	$self->print("    // Generated by astgen\n",
+
-		     "    virtual void visit(Ast${type}* nodep, AstNUser*) {\n",
+	# We need to deal with two cases. For short circuited functions we
-		     "	nodep->iterateChildren(*this);\n",
+	# evaluate the LHS, then apply the short-circuit matches, then
-		     @out_for_type,
+	# evaluate the RHS and possibly THS (ternary operators may
-		     "    }\n",) if $out_for_type[0];
+	# short-circuit) and apply all the other matches.
+
+	# For types without short-circuits, we just use iterateChildren, which
+	# saves one comparison.
+	if ($out_for_type_sc[0]) {	# Short-circuited types
+	    $self->print("    // Generated by astgen with short-circuiting\n",
+			 "    virtual void visit(Ast${type}* nodep, AstNUser*) {\n",
+			 "	nodep->lhsp()->iterateAndNext(*this);\n",
+			 @out_for_type_sc);
+	    $self->print("	nodep->rhsp()->iterateAndNext(*this);\n",
+			 "	AstNodeTriop *tnp = nodep->castNodeTriop();\n",
+			 "	if (tnp && tnp->thsp()) tnp->thsp()->iterateAndNext(*this);\n",
+			 @out_for_type,
+			 "    }\n") if ($out_for_type[0]);
+	} elsif ($out_for_type[0]) {	# Other types with something to print
+	    $self->print("    // Generated by astgen\n",
+			 "    virtual void visit(Ast${type}* nodep, AstNUser*) {\n",
+			 "	nodep->iterateChildren(*this);\n",
+			 @out_for_type,
+			 "    }\n");
+	}
     }
 }
 

test_regress/t/t_gen_cond_bitrange.pl

@@ -0,0 +1,18 @@
+#!/usr/bin/perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2003 by Wilson Snyder. This program is free software; you can
+# redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+
+compile (
+	 );
+
+execute (
+	 check_finished=>1,
+     );
+
+ok(1);
+1;

test_regress/t/t_gen_cond_bitrange.v

@@ -0,0 +1,139 @@
+// DESCRIPTION: Verilator: Verilog Test for short-circuiting in generate "if"
+//
+// The given generate loops should only access valid bits of mask, since that
+// is defined by SIZE. However since the loop range is larger, this only works
+// if short-circuited evaluation of the generate loop is in place.
+
+// This file ONLY is placed into the Public Domain, for any use, without
+// warranty, 2012 by Jeremy Bennett.
+
+
+`define MAX_SIZE  4
+
+
+module t (/*AUTOARG*/
+   // Inputs
+   clk
+   );
+   input clk;
+
+   // Set the parameters, so that we use a size less than MAX_SIZE
+   test_gen
+     #(.SIZE (2),
+       .MASK (2'b11))
+     i_test_gen (.clk (clk));
+
+   // This is only a compilation test, but for good measure we do one clock
+   // cycle.
+   integer count;
+
+   initial begin
+      count = 0;
+   end
+
+   always @(posedge clk) begin
+      if (count == 1) begin
+	 $write("*-* All Finished *-*\n");
+	 $finish;
+      end
+      else begin
+	 count = count + 1;
+      end
+   end
+
+endmodule // t
+
+
+module test_gen
+
+  #( parameter
+     SIZE = `MAX_SIZE,
+     MASK = `MAX_SIZE'b0)
+
+ (/*AUTOARG*/
+   // Inputs
+   clk
+   );
+
+   input clk;
+
+   // Generate blocks that rely on short-circuiting of the logic to avoid errors.
+   generate
+      genvar g;
+
+      for (g = 0; g < `MAX_SIZE; g = g + 1) begin
+         if ((g < SIZE) && MASK[g]) begin
+	    always @(posedge clk) begin
+`ifdef TEST_VERBOSE
+	       $write ("Logical AND generate if MASK [%1d] = %d\n", g, MASK[g]);
+`endif
+	       if (g >= SIZE) begin
+		  $stop;
+	       end
+	    end
+	 end
+      end
+   endgenerate
+
+   generate
+      for (g = 0; g < `MAX_SIZE; g = g + 1) begin
+         if (!((g >= SIZE) || ~MASK[g])) begin
+	    always @(posedge clk) begin
+`ifdef TEST_VERBOSE
+	       $write ("Logical OR generate if MASK [%1d] = %d\n", g, MASK[g]);
+`endif
+	       if (g >= SIZE) begin
+		  $stop;
+	       end
+	    end
+	 end
+      end
+   endgenerate
+
+   generate
+      for (g = 0; g < `MAX_SIZE; g = g + 1) begin
+         if (!((g < SIZE) -> ~MASK[g])) begin
+	    always @(posedge clk) begin
+`ifdef TEST_VERBOSE
+	       $write ("Logical infer generate if MASK [%1d] = %d\n", g, MASK[g]);
+`endif
+	       if (g >= SIZE) begin
+		  $stop;
+	       end
+	    end
+	 end
+      end
+   endgenerate
+
+   generate
+      for (g = 0; g < `MAX_SIZE; g = g + 1) begin
+         if ( g < SIZE ? MASK[g] : 1'b0) begin
+	    always @(posedge clk) begin
+`ifdef TEST_VERBOSE
+	       $write ("Conditional generate if MASK [%1d] = %d\n", g, MASK[g]);
+`endif
+	       if (g >= SIZE) begin
+		  $stop;
+	       end
+	    end
+	 end
+      end
+   endgenerate
+
+   // The other way round
+   generate
+      for (g = 0; g < `MAX_SIZE; g = g + 1) begin
+         if ( g >= SIZE ? 1'b0 : MASK[g]) begin
+	    always @(posedge clk) begin
+`ifdef TEST_VERBOSE
+	       $write ("Conditional generate if MASK [%1d] = %d\n", g, MASK[g]);
+`endif
+	       if (g >= SIZE) begin
+		  $stop;
+	       end
+	    end
+	 end
+      end
+   endgenerate
+
+endmodule

test_regress/t/t_gen_cond_bitrange_bad.pl

@@ -0,0 +1,22 @@
+#!/usr/bin/perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2003 by Wilson Snyder. This program is free software; you can
+# redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+
+compile (
+	 v_flags2 => ["--lint-only"],
+	 fails=>1,
+	 expect=>
+q{%Error: t/t_gen_cond_bitrange_bad.v:\d+: Selection index out of range: 2:2 outside 1:0
+%Error: t/t_gen_cond_bitrange_bad.v:\d+: Selection index out of range: 2:2 outside 1:0
+%Error: t/t_gen_cond_bitrange_bad.v:\d+: Selection index out of range: 2:2 outside 1:0
+%Error: t/t_gen_cond_bitrange_bad.v:\d+: Selection index out of range: 2:2 outside 1:0
+%Error: Exiting due to .*},
+	 );
+
+ok(1);
+1;

test_regress/t/t_gen_cond_bitrange_bad.v

@@ -0,0 +1,106 @@
+// DESCRIPTION: Verilator: Verilog Test for short-circuiting in generate "if"
+// that should not work.
+//
+// The given generate loops should attempt to access invalid bits of mask and
+// trigger errors.
+// is defined by SIZE. However since the loop range is larger, this only works
+// if short-circuited evaluation of the generate loop is in place.
+
+// This file ONLY is placed into the Public Domain, for any use, without
+// warranty, 2012 by Jeremy Bennett.
+
+
+`define MAX_SIZE  3
+
+
+module t (/*AUTOARG*/
+   // Inputs
+   clk
+   );
+   input clk;
+
+   // Set the parameters, so that we use a size less than MAX_SIZE
+   test_gen
+     #(.SIZE (2),
+       .MASK (2'b11))
+     i_test_gen (.clk (clk));
+
+   // This is only a compilation test, so we can immediately finish
+   always @(posedge clk) begin
+      $write("*-* All Finished *-*\n");
+      $finish;
+   end
+
+endmodule // t
+
+
+module test_gen
+
+  #( parameter
+     SIZE = `MAX_SIZE,
+     MASK = `MAX_SIZE'b0)
+
+ (/*AUTOARG*/
+   // Inputs
+   clk
+   );
+
+   input clk;
+
+   // Generate blocks that all have errors in applying short-circuting to
+   // generate "if" conditionals.
+
+   // Attempt to access invalid bits of MASK in different ways
+   generate
+      genvar g;
+
+      for (g = 0; g < `MAX_SIZE; g = g + 1) begin
+         if ((g < (SIZE + 1)) && MASK[g]) begin
+	    always @(posedge clk) begin
+`ifdef TEST_VERBOSE
+	       $write ("Logical AND generate if MASK [%1d] = %d\n", g, MASK[g]);
+`endif
+	    end
+	 end
+      end
+   endgenerate
+
+   generate
+      for (g = 0; g < `MAX_SIZE; g = g + 1) begin
+         if ((g < SIZE) && MASK[g + 1]) begin
+	    always @(posedge clk) begin
+`ifdef TEST_VERBOSE
+	       $write ("Logical AND generate if MASK [%1d] = %d\n", g, MASK[g]);
+`endif
+	    end
+	 end
+      end
+   endgenerate
+
+   // Attempt to short-circuit bitwise AND
+   generate
+      for (g = 0; g < `MAX_SIZE; g = g + 1) begin
+         if ((g < (SIZE)) & MASK[g]) begin
+	    always @(posedge clk) begin
+`ifdef TEST_VERBOSE
+	       $write ("Bitwise AND generate if MASK [%1d] = %d\n", g, MASK[g]);
+`endif
+	    end
+	 end
+      end
+   endgenerate
+
+   // Attempt to short-circuit bitwise OR
+   generate
+      for (g = 0; g < `MAX_SIZE; g = g + 1) begin
+         if (!((g >= SIZE) | ~MASK[g])) begin
+	    always @(posedge clk) begin
+`ifdef TEST_VERBOSE
+	       $write ("Bitwise OR generate if MASK [%1d] = %d\n", g, MASK[g]);
+`endif
+	    end
+	 end
+      end
+   endgenerate
+
+endmodule