Commit bc18e93825

Evan Haas <evan@lagerdata.com>
2021-08-02 17:15:59
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.
1 parent 41d7787
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
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;
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<const clang::IntegerLiteral *>(self);
     auto casted_ctx = reinterpret_cast<const clang::ASTContext *>(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;
 }
 
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 *);
 
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))];
+    });
 }