From bc18e93825f1f3703b97590cc0e963b3faa8cd3e Mon Sep 17 00:00:00 2001 From: Evan Haas Date: Mon, 2 Aug 2021 08:15:59 -0700 Subject: [PATCH] translate-c: better codegen for pointer index by int literal #8589 introduced correct handling of signed (possibly negative) array access of pointers. Since unadorned integer literals in C are signed, this resulted in inefficient generated code when indexing a pointer by a non-negative integer literal. --- src/clang.zig | 4 ++-- src/translate_c.zig | 20 +++++++++++++++++--- src/zig_clang.cpp | 14 ++++++++++++-- src/zig_clang.h | 2 +- test/translate_c.zig | 11 +++++++++++ 5 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/clang.zig b/src/clang.zig index 0632c5ab67..1254a505f4 100644 --- a/src/clang.zig +++ b/src/clang.zig @@ -616,8 +616,8 @@ pub const IntegerLiteral = opaque { pub const getBeginLoc = ZigClangIntegerLiteral_getBeginLoc; extern fn ZigClangIntegerLiteral_getBeginLoc(*const IntegerLiteral) SourceLocation; - pub const isZero = ZigClangIntegerLiteral_isZero; - extern fn ZigClangIntegerLiteral_isZero(*const IntegerLiteral, *bool, *const ASTContext) bool; + pub const getSignum = ZigClangIntegerLiteral_getSignum; + extern fn ZigClangIntegerLiteral_getSignum(*const IntegerLiteral, *c_int, *const ASTContext) bool; }; /// This is just used as a namespace for a static method on clang's Lexer class; we don't directly diff --git a/src/translate_c.zig b/src/translate_c.zig index 8b465fff96..e6d6392a9c 100644 --- a/src/translate_c.zig +++ b/src/translate_c.zig @@ -1985,10 +1985,11 @@ fn transBoolExpr( used: ResultUsed, ) TransError!Node { if (@ptrCast(*const clang.Stmt, expr).getStmtClass() == .IntegerLiteralClass) { - var is_zero: bool = undefined; - if (!(@ptrCast(*const clang.IntegerLiteral, expr).isZero(&is_zero, c.clang_context))) { + var signum: c_int = undefined; + if (!(@ptrCast(*const clang.IntegerLiteral, expr).getSignum(&signum, c.clang_context))) { return fail(c, error.UnsupportedTranslation, expr.getBeginLoc(), "invalid integer literal", .{}); } + const is_zero = signum == 0; return Node{ .tag_if_small_enough = @enumToInt(([2]Tag{ .true_literal, .false_literal })[@boolToInt(is_zero)]) }; } @@ -3360,6 +3361,7 @@ fn transArrayAccess(c: *Context, scope: *Scope, stmt: *const clang.ArraySubscrip const subscr_qt = getExprQualType(c, subscr_expr); const is_longlong = cIsLongLongInteger(subscr_qt); const is_signed = cIsSignedInteger(subscr_qt); + const is_nonnegative_int_literal = cIsNonNegativeIntLiteral(c, subscr_expr); // Unwrap the base statement if it's an array decayed to a bare pointer type // so that we index the array itself @@ -3374,7 +3376,7 @@ fn transArrayAccess(c: *Context, scope: *Scope, stmt: *const clang.ArraySubscrip // Special case: actual pointer (not decayed array) and signed integer subscript // See discussion at https://github.com/ziglang/zig/pull/8589 - if (is_signed and (base_stmt == unwrapped_base) and !is_vector) return transSignedArrayAccess(c, scope, base_stmt, subscr_expr, result_used); + if (is_signed and (base_stmt == unwrapped_base) and !is_vector and !is_nonnegative_int_literal) return transSignedArrayAccess(c, scope, base_stmt, subscr_expr, result_used); const container_node = try transExpr(c, scope, unwrapped_base, .used); const rhs = if (is_longlong or is_signed) blk: { @@ -4260,6 +4262,18 @@ fn cIntTypeCmp(a: clang.QualType, b: clang.QualType) math.Order { return math.order(a_index, b_index); } +/// Checks if expr is an integer literal >= 0 +fn cIsNonNegativeIntLiteral(c: *Context, expr: *const clang.Expr) bool { + if (@ptrCast(*const clang.Stmt, expr).getStmtClass() == .IntegerLiteralClass) { + var signum: c_int = undefined; + if (!(@ptrCast(*const clang.IntegerLiteral, expr).getSignum(&signum, c.clang_context))) { + return false; + } + return signum >= 0; + } + return false; +} + fn cIsSignedInteger(qt: clang.QualType) bool { const c_type = qualTypeCanon(qt); if (c_type.getTypeClass() != .Builtin) return false; diff --git a/src/zig_clang.cpp b/src/zig_clang.cpp index 4fe1dfc286..6611585f68 100644 --- a/src/zig_clang.cpp +++ b/src/zig_clang.cpp @@ -2754,7 +2754,7 @@ struct ZigClangSourceLocation ZigClangIntegerLiteral_getBeginLoc(const struct Zi return bitcast(casted->getBeginLoc()); } -bool ZigClangIntegerLiteral_isZero(const struct ZigClangIntegerLiteral *self, bool *result, const struct ZigClangASTContext *ctx) { +bool ZigClangIntegerLiteral_getSignum(const struct ZigClangIntegerLiteral *self, int *result, const struct ZigClangASTContext *ctx) { auto casted_self = reinterpret_cast(self); auto casted_ctx = reinterpret_cast(ctx); clang::Expr::EvalResult eval_result; @@ -2763,7 +2763,17 @@ bool ZigClangIntegerLiteral_isZero(const struct ZigClangIntegerLiteral *self, bo } const llvm::APSInt result_int = eval_result.Val.getInt(); const llvm::APSInt zero(result_int.getBitWidth(), result_int.isUnsigned()); - *result = zero == result_int; + + if (zero == result_int) { + *result = 0; + } else if (result_int < zero) { + *result = -1; + } else if (result_int > zero) { + *result = 1; + } else { + return false; + } + return true; } diff --git a/src/zig_clang.h b/src/zig_clang.h index dc35df3772..0e7a8b2990 100644 --- a/src/zig_clang.h +++ b/src/zig_clang.h @@ -1222,7 +1222,7 @@ ZIG_EXTERN_C struct ZigClangQualType ZigClangCStyleCastExpr_getType(const struct ZIG_EXTERN_C bool ZigClangIntegerLiteral_EvaluateAsInt(const struct ZigClangIntegerLiteral *, struct ZigClangExprEvalResult *, const struct ZigClangASTContext *); ZIG_EXTERN_C struct ZigClangSourceLocation ZigClangIntegerLiteral_getBeginLoc(const struct ZigClangIntegerLiteral *); -ZIG_EXTERN_C bool ZigClangIntegerLiteral_isZero(const struct ZigClangIntegerLiteral *, bool *, const struct ZigClangASTContext *); +ZIG_EXTERN_C bool ZigClangIntegerLiteral_getSignum(const struct ZigClangIntegerLiteral *, int *, const struct ZigClangASTContext *); ZIG_EXTERN_C const struct ZigClangExpr *ZigClangReturnStmt_getRetValue(const struct ZigClangReturnStmt *); diff --git a/test/translate_c.zig b/test/translate_c.zig index b75d3b5cac..6ddc2107ee 100644 --- a/test/translate_c.zig +++ b/test/translate_c.zig @@ -3630,4 +3630,15 @@ pub fn addCases(cases: *tests.TranslateCContext) void { , &[_][]const u8{ \\pub const FOO = @import("std").zig.c_translation.Macros.U_SUFFIX; }); + + cases.add("Simple array access of pointer with non-negative integer constant", + \\void foo(int *p) { + \\ p[0]; + \\ p[1]; + \\} + , &[_][]const u8{ + \\_ = p[@intCast(c_uint, @as(c_int, 0))]; + , + \\_ = p[@intCast(c_uint, @as(c_int, 1))]; + }); }