Skip to content

Commit

Permalink
Fix == and != when comparing signed vs unsigned integers
Browse files Browse the repository at this point in the history
  • Loading branch information
asterite committed Sep 10, 2018
1 parent 1cbafcb commit cc2ab58
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 15 deletions.
2 changes: 1 addition & 1 deletion spec/std/float_printer/diy_fp_spec.cr
Expand Up @@ -173,6 +173,6 @@ describe DiyFP do
fp = DiyFP.from_f_normalized(f)

fp.exp.should eq 0x7FE - 0x3FF - 52 - 11
fp.frac.should eq 0x001fffffffffffff << 11
fp.frac.should eq 0x001fffffffffffff_u64 << 11
end
end
12 changes: 12 additions & 0 deletions spec/std/int_spec.cr
Expand Up @@ -537,6 +537,18 @@ describe "Int" do
end
end

{% if compare_versions(Crystal::VERSION, "2.6.1") > 0 %}
it "compares equality and inequality of signed vs. unsigned integers" do
x = -1
y = x.unsafe_as(UInt32)

(x == y).should be_false
(y == x).should be_false
(x != y).should be_true
(y != x).should be_true
end
{% end %}

it "clones" do
[1_u8, 2_u16, 3_u32, 4_u64, 5_i8, 6_i16, 7_i32, 8_i64].each do |value|
value.clone.should eq(value)
Expand Down
64 changes: 62 additions & 2 deletions src/compiler/crystal/codegen/primitives.cr
Expand Up @@ -118,6 +118,8 @@ class Crystal::CodeGenVisitor
when "<=" then return codegen_binary_op_lte(t1, t2, p1, p2)
when ">" then return codegen_binary_op_gt(t1, t2, p1, p2)
when ">=" then return codegen_binary_op_gte(t1, t2, p1, p2)
when "==" then return codegen_binary_op_eq(t1, t2, p1, p2)
when "!=" then return codegen_binary_op_ne(t1, t2, p1, p2)
end

p1, p2 = codegen_binary_extend_int(t1, t2, p1, p2)
Expand All @@ -133,8 +135,6 @@ class Crystal::CodeGenVisitor
when "|" then codegen_trunc_binary_op_result(t1, t2, or(p1, p2))
when "&" then codegen_trunc_binary_op_result(t1, t2, and(p1, p2))
when "^" then codegen_trunc_binary_op_result(t1, t2, builder.xor(p1, p2))
when "==" then builder.icmp(LLVM::IntPredicate::EQ, p1, p2)
when "!=" then builder.icmp(LLVM::IntPredicate::NE, p1, p2)
else raise "BUG: trying to codegen #{t1} #{op} #{t2}"
end
end
Expand All @@ -161,6 +161,26 @@ class Crystal::CodeGenVisitor
end
end

# The below methods (lt, lte, gt, gte, eq, ne) perform
# comparisons on two integers x and y,
# where t1, t2 are their types and p1, p2 are their values.
#
# In LLVM, Int32 and UInt32 are represented as the same type
# (i32) and although integer operations have a sign
# (SGE, UGE, signed/unsigned greater than or equal)
# when we have one signed integer and one unsigned integer
# we can't choose a signedness for the operation. In that
# case we need to perform some additional checks.
#
# Equality and inequality operations for integers in LLVM don't have
# signedness, they just compare bit patterns. But for example
# the Int32 with value -1 and the UInt32 with value
# 4294967295 have the same bit pattern, and yet they are not
# equal, so again we must perform some additional checks
# (mainly, if the signed value is negative then there's
# no way they are equal, and for positive values we can
# perform the usual bit equality).

def codegen_binary_op_lt(t1, t2, p1, p2)
if t1.signed? == t2.signed?
p1, p2 = codegen_binary_extend_int(t1, t2, p1, p2)
Expand Down Expand Up @@ -313,6 +333,46 @@ class Crystal::CodeGenVisitor
end
end

def codegen_binary_op_eq(t1, t2, p1, p2)
p1, p2 = codegen_binary_extend_int(t1, t2, p1, p2)

if t1.signed? == t2.signed?
builder.icmp(LLVM::IntPredicate::EQ, p1, p2)
elsif t1.signed? && t2.unsigned?
# x >= 0 && x == y
and(
builder.icmp(LLVM::IntPredicate::SGE, p1, p1.type.const_int(0)),
builder.icmp(LLVM::IntPredicate::EQ, p1, p2)
)
else # t1.unsigned? && t2.signed?
# y >= 0 && x == y
and(
builder.icmp(LLVM::IntPredicate::SGE, p2, p2.type.const_int(0)),
builder.icmp(LLVM::IntPredicate::EQ, p1, p2)
)
end
end

def codegen_binary_op_ne(t1, t2, p1, p2)
p1, p2 = codegen_binary_extend_int(t1, t2, p1, p2)

if t1.signed? == t2.signed?
builder.icmp(LLVM::IntPredicate::NE, p1, p2)
elsif t1.signed? && t2.unsigned?
# x < 0 || x != y
or(
builder.icmp(LLVM::IntPredicate::SLT, p1, p1.type.const_int(0)),
builder.icmp(LLVM::IntPredicate::NE, p1, p2)
)
else # t1.unsigned? && t2.signed?
# y < 0 || x != y
or(
builder.icmp(LLVM::IntPredicate::SLT, p2, p2.type.const_int(0)),
builder.icmp(LLVM::IntPredicate::NE, p1, p2)
)
end
end

def codegen_binary_op(op, t1 : IntegerType, t2 : FloatType, p1, p2)
p1 = codegen_cast(t1, t2, p1)
codegen_binary_op(op, t2, t2, p1, p2)
Expand Down
2 changes: 2 additions & 0 deletions src/iconv.cr
Expand Up @@ -4,6 +4,8 @@ require "c/iconv"
struct Iconv
@skip_invalid : Bool

ERROR = LibC::SizeT::MAX # (size_t)(-1)

def initialize(from : String, to : String, invalid : Symbol? = nil)
original_from, original_to = from, to

Expand Down
4 changes: 2 additions & 2 deletions src/io/encoding.cr
Expand Up @@ -31,7 +31,7 @@ class IO
outbuf_ptr = outbuf.to_unsafe
outbytesleft = LibC::SizeT.new(outbuf.size)
err = @iconv.convert(pointerof(inbuf_ptr), pointerof(inbytesleft), pointerof(outbuf_ptr), pointerof(outbytesleft))
if err == -1
if err == Iconv::ERROR
@iconv.handle_invalid(pointerof(inbuf_ptr), pointerof(inbytesleft))
end
io.write(outbuf.to_slice[0, outbuf.size - outbytesleft])
Expand Down Expand Up @@ -95,7 +95,7 @@ class IO
@out_slice = @out_buffer[0, OUT_BUFFER_SIZE - out_buffer_left]

# Check for errors
if result == -1
if result == Iconv::ERROR
case Errno.value
when Errno::EILSEQ
# For an illegal sequence we just skip one byte and we'll continue next
Expand Down
22 changes: 13 additions & 9 deletions src/llvm/generic_value.cr
Expand Up @@ -2,31 +2,35 @@ class LLVM::GenericValue
def initialize(@unwrap : LibLLVM::GenericValueRef, @context : LLVM::Context)
end

def to_i
LibLLVM.generic_value_to_int(self, 1)
def to_i : Int32
to_i64.to_i32
end

def to_u64
to_i
def to_i64 : Int64
LibLLVM.generic_value_to_int(self, signed: 1).unsafe_as(Int64)
end

def to_b
def to_u64 : UInt64
LibLLVM.generic_value_to_int(self, signed: 0)
end

def to_b : Bool
to_i != 0
end

def to_f32
def to_f32 : Float32
LibLLVM.generic_value_to_float(@context.float, self)
end

def to_f64
def to_f64 : Float64
LibLLVM.generic_value_to_float(@context.double, self)
end

def to_string
def to_string : String
to_pointer.as(String)
end

def to_pointer
def to_pointer : Void*
LibLLVM.generic_value_to_pointer(self)
end

Expand Down
2 changes: 1 addition & 1 deletion src/string.cr
Expand Up @@ -1175,7 +1175,7 @@ class String
outbuf_ptr = outbuf.to_unsafe
outbytesleft = LibC::SizeT.new(outbuf.size)
err = iconv.convert(pointerof(inbuf_ptr), pointerof(inbytesleft), pointerof(outbuf_ptr), pointerof(outbytesleft))
if err == -1
if err == Iconv::ERROR
iconv.handle_invalid(pointerof(inbuf_ptr), pointerof(inbytesleft))
end
io.write(outbuf.to_slice[0, outbuf.size - outbytesleft])
Expand Down

0 comments on commit cc2ab58

Please sign in to comment.