mirror of
				https://github.com/YosysHQ/yosys
				synced 2025-11-03 21:09:12 +00:00 
			
		
		
		
	Merge pull request #5101 from georgerennie/george/opt_expr_shift_ovfl
opt_expr: fix shift optimization with overflowing shift amount
This commit is contained in:
		
						commit
						6331f92d00
					
				
					 4 changed files with 168 additions and 2 deletions
				
			
		| 
						 | 
				
			
			@ -380,6 +380,33 @@ int RTLIL::Const::as_int(bool is_signed) const
 | 
			
		|||
	return ret;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
bool RTLIL::Const::convertible_to_int(bool is_signed) const
 | 
			
		||||
{
 | 
			
		||||
	auto size = get_min_size(is_signed);
 | 
			
		||||
	return (size > 0 && size <= 32);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
std::optional<int> RTLIL::Const::try_as_int(bool is_signed) const
 | 
			
		||||
{
 | 
			
		||||
	if (!convertible_to_int(is_signed))
 | 
			
		||||
		return std::nullopt;
 | 
			
		||||
	return as_int(is_signed);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
int RTLIL::Const::as_int_saturating(bool is_signed) const
 | 
			
		||||
{
 | 
			
		||||
	if (!convertible_to_int(is_signed)) {
 | 
			
		||||
		if (!is_signed)
 | 
			
		||||
			return std::numeric_limits<int>::max();
 | 
			
		||||
 | 
			
		||||
		const auto min_size = get_min_size(is_signed);
 | 
			
		||||
		log_assert(min_size > 0);
 | 
			
		||||
		const auto neg = get_bits().at(min_size - 1);
 | 
			
		||||
		return neg ? std::numeric_limits<int>::min() : std::numeric_limits<int>::max();
 | 
			
		||||
	}
 | 
			
		||||
	return as_int(is_signed);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
int RTLIL::Const::get_min_size(bool is_signed) const
 | 
			
		||||
{
 | 
			
		||||
	if (empty()) return 0;
 | 
			
		||||
| 
						 | 
				
			
			@ -5462,6 +5489,38 @@ int RTLIL::SigSpec::as_int(bool is_signed) const
 | 
			
		|||
	return 0;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
bool RTLIL::SigSpec::convertible_to_int(bool is_signed) const
 | 
			
		||||
{
 | 
			
		||||
	cover("kernel.rtlil.sigspec.convertible_to_int");
 | 
			
		||||
 | 
			
		||||
	pack();
 | 
			
		||||
	if (!is_fully_const())
 | 
			
		||||
		return false;
 | 
			
		||||
 | 
			
		||||
	return RTLIL::Const(chunks_[0].data).convertible_to_int(is_signed);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
std::optional<int> RTLIL::SigSpec::try_as_int(bool is_signed) const
 | 
			
		||||
{
 | 
			
		||||
	cover("kernel.rtlil.sigspec.try_as_int");
 | 
			
		||||
 | 
			
		||||
	pack();
 | 
			
		||||
	if (!is_fully_const())
 | 
			
		||||
		return std::nullopt;
 | 
			
		||||
 | 
			
		||||
	return RTLIL::Const(chunks_[0].data).try_as_int(is_signed);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
int RTLIL::SigSpec::as_int_saturating(bool is_signed) const
 | 
			
		||||
{
 | 
			
		||||
	cover("kernel.rtlil.sigspec.try_as_int");
 | 
			
		||||
 | 
			
		||||
	pack();
 | 
			
		||||
	log_assert(is_fully_const() && GetSize(chunks_) <= 1);
 | 
			
		||||
	log_assert(!empty());
 | 
			
		||||
	return RTLIL::Const(chunks_[0].data).as_int_saturating(is_signed);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
std::string RTLIL::SigSpec::as_string() const
 | 
			
		||||
{
 | 
			
		||||
	cover("kernel.rtlil.sigspec.as_string");
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -753,7 +753,26 @@ public:
 | 
			
		|||
 | 
			
		||||
	std::vector<RTLIL::State>& bits();
 | 
			
		||||
	bool as_bool() const;
 | 
			
		||||
 | 
			
		||||
	// Convert the constant value to a C++ int.
 | 
			
		||||
	// NOTE: If the constant is too wide to fit in int (32 bits) this will
 | 
			
		||||
	// truncate any higher bits, potentially over/underflowing. Consider using
 | 
			
		||||
	// try_as_int, as_int_saturating, or guarding behind convertible_to_int
 | 
			
		||||
	// instead.
 | 
			
		||||
	int as_int(bool is_signed = false) const;
 | 
			
		||||
 | 
			
		||||
	// Returns true iff the constant can be converted to an int without
 | 
			
		||||
	// over/underflow.
 | 
			
		||||
	bool convertible_to_int(bool is_signed = false) const;
 | 
			
		||||
 | 
			
		||||
	// Returns the constant's value as an int if it can be represented without
 | 
			
		||||
	// over/underflow, or std::nullopt otherwise.
 | 
			
		||||
	std::optional<int> try_as_int(bool is_signed = false) const;
 | 
			
		||||
 | 
			
		||||
	// Returns the constant's value as an int if it can be represented without
 | 
			
		||||
	// over/underflow, otherwise the max/min value for int depending on the sign.
 | 
			
		||||
	int as_int_saturating(bool is_signed = false) const;
 | 
			
		||||
 | 
			
		||||
	std::string as_string(const char* any = "-") const;
 | 
			
		||||
	static Const from_string(const std::string &str);
 | 
			
		||||
	std::vector<RTLIL::State> to_bits() const;
 | 
			
		||||
| 
						 | 
				
			
			@ -1130,7 +1149,27 @@ public:
 | 
			
		|||
	bool is_onehot(int *pos = nullptr) const;
 | 
			
		||||
 | 
			
		||||
	bool as_bool() const;
 | 
			
		||||
 | 
			
		||||
	// Convert the SigSpec to a C++ int, assuming all bits are constant.
 | 
			
		||||
	// NOTE: If the value is too wide to fit in int (32 bits) this will
 | 
			
		||||
	// truncate any higher bits, potentially over/underflowing. Consider using
 | 
			
		||||
	// try_as_int, as_int_saturating, or guarding behind convertible_to_int
 | 
			
		||||
	// instead.
 | 
			
		||||
	int as_int(bool is_signed = false) const;
 | 
			
		||||
 | 
			
		||||
	// Returns true iff the SigSpec is constant and can be converted to an int
 | 
			
		||||
	// without over/underflow.
 | 
			
		||||
	bool convertible_to_int(bool is_signed = false) const;
 | 
			
		||||
 | 
			
		||||
	// Returns the SigSpec's value as an int if it is a constant and can be
 | 
			
		||||
	// represented without over/underflow, or std::nullopt otherwise.
 | 
			
		||||
	std::optional<int> try_as_int(bool is_signed = false) const;
 | 
			
		||||
 | 
			
		||||
	// Returns an all constant SigSpec's value as an int if it can be represented
 | 
			
		||||
	// without over/underflow, otherwise the max/min value for int depending on
 | 
			
		||||
	// the sign.
 | 
			
		||||
	int as_int_saturating(bool is_signed = false) const;
 | 
			
		||||
 | 
			
		||||
	std::string as_string() const;
 | 
			
		||||
	RTLIL::Const as_const() const;
 | 
			
		||||
	RTLIL::Wire *as_wire() const;
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -1307,7 +1307,12 @@ skip_fine_alu:
 | 
			
		|||
		if (cell->type.in(ID($shl), ID($shr), ID($sshl), ID($sshr), ID($shift), ID($shiftx)) && (keepdc ? assign_map(cell->getPort(ID::B)).is_fully_def() : assign_map(cell->getPort(ID::B)).is_fully_const()))
 | 
			
		||||
		{
 | 
			
		||||
			bool sign_ext = cell->type == ID($sshr) && cell->getParam(ID::A_SIGNED).as_bool();
 | 
			
		||||
			int shift_bits = assign_map(cell->getPort(ID::B)).as_int(cell->type.in(ID($shift), ID($shiftx)) && cell->getParam(ID::B_SIGNED).as_bool());
 | 
			
		||||
			RTLIL::SigSpec sig_b = assign_map(cell->getPort(ID::B));
 | 
			
		||||
			const bool b_sign_ext = cell->type.in(ID($shift), ID($shiftx)) && cell->getParam(ID::B_SIGNED).as_bool();
 | 
			
		||||
			// We saturate the value to prevent overflow, but note that this could
 | 
			
		||||
			// cause incorrect opimization in the impractical case that A is 2^32 bits
 | 
			
		||||
			// wide
 | 
			
		||||
			int shift_bits = sig_b.as_int_saturating(b_sign_ext);
 | 
			
		||||
 | 
			
		||||
			if (cell->type.in(ID($shl), ID($sshl)))
 | 
			
		||||
				shift_bits *= -1;
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -20,7 +20,11 @@ module top (
 | 
			
		|||
	output wire        [7:0] sshr_uu,
 | 
			
		||||
	output wire signed [7:0] sshr_us,
 | 
			
		||||
	output wire        [7:0] sshr_su,
 | 
			
		||||
	output wire signed [7:0] sshr_ss
 | 
			
		||||
	output wire signed [7:0] sshr_ss,
 | 
			
		||||
	output wire        [7:0] shiftx_uu,
 | 
			
		||||
	output wire signed [7:0] shiftx_us,
 | 
			
		||||
	output wire        [7:0] shiftx_su,
 | 
			
		||||
	output wire signed [7:0] shiftx_ss
 | 
			
		||||
);
 | 
			
		||||
	assign  shl_uu = in_u << 20;
 | 
			
		||||
	assign  shl_us = in_u << 20;
 | 
			
		||||
| 
						 | 
				
			
			@ -38,9 +42,20 @@ module top (
 | 
			
		|||
	assign sshr_us = in_u >>> 20;
 | 
			
		||||
	assign sshr_su = in_s >>> 20;
 | 
			
		||||
	assign sshr_ss = in_s >>> 20;
 | 
			
		||||
	wire [7:0] shamt = 20;
 | 
			
		||||
	assign shiftx_uu = in_u[shamt +: 8];
 | 
			
		||||
	assign shiftx_us = in_u[shamt +: 8];
 | 
			
		||||
	assign shiftx_su = in_s[shamt +: 8];
 | 
			
		||||
	assign shiftx_ss = in_s[shamt +: 8];
 | 
			
		||||
endmodule
 | 
			
		||||
EOT
 | 
			
		||||
 | 
			
		||||
select -assert-count 4 t:$shl
 | 
			
		||||
select -assert-count 4 t:$shr
 | 
			
		||||
select -assert-count 4 t:$sshl
 | 
			
		||||
select -assert-count 4 t:$sshr
 | 
			
		||||
select -assert-count 4 t:$shiftx
 | 
			
		||||
 | 
			
		||||
equiv_opt opt_expr
 | 
			
		||||
 | 
			
		||||
design -load postopt
 | 
			
		||||
| 
						 | 
				
			
			@ -48,3 +63,51 @@ select -assert-none t:$shl
 | 
			
		|||
select -assert-none t:$shr
 | 
			
		||||
select -assert-none t:$sshl
 | 
			
		||||
select -assert-none t:$sshr
 | 
			
		||||
select -assert-none t:$shiftx
 | 
			
		||||
 | 
			
		||||
design -reset
 | 
			
		||||
 | 
			
		||||
read_verilog <<EOT
 | 
			
		||||
module top (
 | 
			
		||||
	input  wire        [3:0]  in,
 | 
			
		||||
	output wire        [7:0]  shl,
 | 
			
		||||
	output wire        [7:0]  shr,
 | 
			
		||||
	output wire        [7:0] sshl,
 | 
			
		||||
	output wire        [7:0] sshr,
 | 
			
		||||
	output wire        [7:0] shiftx,
 | 
			
		||||
 | 
			
		||||
	output wire        [7:0]  shl_s,
 | 
			
		||||
	output wire        [7:0]  shr_s,
 | 
			
		||||
	output wire        [7:0] sshl_s,
 | 
			
		||||
	output wire        [7:0] sshr_s,
 | 
			
		||||
	output wire        [7:0] shiftx_s,
 | 
			
		||||
);
 | 
			
		||||
	assign  shl = in << 36'hfffffffff;
 | 
			
		||||
	assign  shr = in >> 36'hfffffffff;
 | 
			
		||||
	assign sshl = in <<< 36'hfffffffff;
 | 
			
		||||
	assign sshr = in >>> 36'hfffffffff;
 | 
			
		||||
	assign shiftx = in[36'hfffffffff +: 8];
 | 
			
		||||
 | 
			
		||||
	wire signed [35:0] shamt = 36'hfffffffff;
 | 
			
		||||
	assign  shl_s = in << shamt;
 | 
			
		||||
	assign  shr_s = in >> shamt;
 | 
			
		||||
	assign sshl_s = in <<< shamt;
 | 
			
		||||
	assign sshr_s = in >>> shamt;
 | 
			
		||||
	assign shiftx_s = in[shamt +: 8];
 | 
			
		||||
endmodule
 | 
			
		||||
EOT
 | 
			
		||||
 | 
			
		||||
select -assert-count 2 t:$shl
 | 
			
		||||
select -assert-count 2 t:$shr
 | 
			
		||||
select -assert-count 2 t:$sshl
 | 
			
		||||
select -assert-count 2 t:$sshr
 | 
			
		||||
select -assert-count 1 t:$shiftx
 | 
			
		||||
 | 
			
		||||
equiv_opt opt_expr
 | 
			
		||||
 | 
			
		||||
design -load postopt
 | 
			
		||||
select -assert-none t:$shl
 | 
			
		||||
select -assert-none t:$shr
 | 
			
		||||
select -assert-none t:$sshl
 | 
			
		||||
select -assert-none t:$sshr
 | 
			
		||||
select -assert-none t:$shiftx
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue