Skip to content

translate-c: emit alignCast when casting pointers #2316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from

Conversation

raulgrell
Copy link
Contributor

This small patch makes all pointer casts include an align cast as well. This isn't great, but I haven't quite worked out how to work out when they're needed. Tests still need to be changed.

Is there a simple way of running only the translate-c tests?

@LemonBoy
Copy link
Contributor

This small patch makes all pointer casts include an align cast as well. This isn't great, but I haven't quite worked out how to work out when they're needed.

I think you can use getTypeAlignIfKnown and only emit a @alignCast if the pointee (is that even a word) types alignment differ.

unsigned ZigClangASTContext_getTypeAlignIfKnown(const ZigClangASTContext* self, ZigClangQualType T) {
    return reinterpret_cast<const clang::ASTContext *>(self)->getTypeAlignIfKnown(bitcast(T));
}

Is there a simple way of running only the translate-c tests?

./zig build --build-file ../build.zig test-translate-c

@raulgrell
Copy link
Contributor Author

raulgrell commented Apr 20, 2019

@LemonBoy Thanks, that really helped! I still need to trigger a test case, but in theory this should be enough. The other places where ptrCast is emitted don't seem to be an issue.

I'll let you know once I've got some tests

EDIT: Nope, I'm not really doing the right thing. I'll try again tonight.

@raulgrell
Copy link
Contributor Author

Turns out I was checking the alignment of the pointer and not the pointee... At this point, this handles the following test code if you ignore the c alignment attributes:

typedef unsigned char U8;
typedef unsigned short __attribute__((aligned(1))) U16;
typedef unsigned int __attribute__((aligned(1))) U32;

void main() {
    U8 a;
    U8 b;
    U16 c;

    void * pa = (U8 *)(&a);
    void * px = (U16 *)(&a);
    U32 * pxx = (U16 *)(&a);
    void * ppx = (U32 *)(&a);
}

generating

pub const U8 = u8;
pub const U16 = c_ushort;
pub const U32 = c_uint;
pub export fn main() void {
    var a: U8 = undefined;
    var b: U8 = undefined;
    var c: U16 = undefined;
    var pa: ?*c_void = @ptrCast(?*c_void, &a);
    var px: ?*c_void = @ptrCast(?*c_void, @ptrCast([*c]U16, @alignCast(@alignOf([*c]U16), &a)));
    var pxx: [*c]U32 = @ptrCast([*c]U32, @alignCast(@alignOf([*c]U32), @ptrCast([*c]U16, @alignCast(@alignOf([*c]U16), &a))));
    var ppx: ?*c_void = @ptrCast(?*c_void, @ptrCast([*c]U32, @alignCast(@alignOf([*c]U32), &a)));
}

Once translate-c can handle alignment attributes, it should skip the alignCast

@raulgrell
Copy link
Contributor Author

Questions:

  • If the bit size is known, should it be included directly instead of calling @Alignof on the target type?
  • Should I extract the alignment comparison into a function? Or is the duplication ok?
  • If the alignment is not known, should the alignCasts be skipped?

@raulgrell
Copy link
Contributor Author

@LemonBoy's implementation in #2318 is more robust than this one. The only practical difference is hardcoding the alignment vs calling alignOf, which was already mentioned in the comments of his PR.

@raulgrell raulgrell closed this Apr 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants