Overview

• Motivation
• What is repr align?
• Producing a PR
• Completing a PR
• Closing thoughts

Why did I want to do this?

• Enthusiastic about Open Source.
• Enthusiastic about Rust.
• Some time off work over summer.
• Work on game engines in C++ by day.
• Maybe I could help make Rust better for game engine developers?
• Maybe contributing would be more interesting than my hobby projects?

Finding a task

• What is Rust missing that game engines need?
• What is small enough that I could tackle?
• What isn't being worked on already?

[repr(align)]

• RFC 1358 adds support for specifying custom alignment on structs.
• Custom alignment is a very niche requirement.
• But when you need it, it's pretty annoying to deal with manually.
• Some hardware and APIs require aligned memory.
• Align to cache line (e.g. avoid false sharing).
• LLVM should do most of the work right?
• No one else appeared to be working on it.
• I had some prior experience with LLVM, seems achievable.

What is alignment anyway

• Most of the time, people don't need to know about it.
• "Hardware" reads/writes memory in word sized chunks (e.g. 4 bytes on 32 bit) or larger.
• Data alignment means putting the data at a memory address equal to some multiple of the word size.
• Padding fields are added by compilers to ensure data is aligned correctly.
• Reading/writing at less granularity either doesn't work or is slower depending on hardware.
• One example is vector instructions on x86_64 will segfault if given an unaligned address (whoops!).

Primitive alignment

#[repr(C)]
struct S {
  field1: i8,  // offset 0, 1 byte aligned
  // 3 bytes padding
  field2: i32, // offset 4, 4 byte aligned
  // 0 bytes padding
  field3: i16, // offset 8, 2 byte aligned
  // 2 bytes padding
} // struct is 12 bytes and 4 byte aligned

This is how a struct is aligned and padded without custom alignment on x86.

Primitive alignment

• LLVM automatically determines padding between fields and at the end of the struct.
• LLVM ensures this is allocated on the stack at the right alignment.
• Rust allocator ensures this is allocated on the heap at the right alignment.

Field reordering

struct S {
  field1: i8,  // offset 6, 1 byte aligned
  field2: i32, // offset 0, 4 byte aligned
  field3: i16, // offset 4, 2 byte aligned
  // 1 bytes padding
} // struct is 8 bytes and 4 byte aligned

• Rust reserves the right to reorder struct fields for better layout and size.
• This is happening in nightly now.
• [repr(C)] disables reordering and means the struct can be used with FFI.

Custom alignment in Rust

#![feature(attr_literals)]
#![feature(repr_align)]
#[repr(align(16))]
struct Align16(i32);
struct Nested {
  field1: i32,     // offset 1, 4 byte aligned
  field2: Align16, // offset 0, 16 byte aligned
  // 12 bytes padding
} // struct is 32 bytes and 16 byte aligned

Notice the offsets in Nested, this struct layout has been re-ordered by Rust.

This should just work TM

• LLVM handles alignment right?
• So just parse [repr(align)] tell LLVM about it and write some tests.
• Job done!

Where to start

#[repr(align = "16")]
struct Align16(i32);
assert_eq!(mem::align_of::<Align16>(), 16);
assert_eq!(mem::size_of::<Align16>(), 16);

• RFC has some example code, try make that work.
• Look at other uses of repr to work out how to parse.
• How to introspect a compiler?
• Spend many days println debugging rustc to understand what it's doing.
• It takes 15 minutes at best to build stage 1 rustc on my laptop.
• A lot of time with very little forward progress at this point!

Working on Rust

• Read CONTRIBUTING.md
• If you have already read it, it might have changed!
• Compiler builds in multiple stages, 0 to 2, using previous stage to build the next. See src/bootstrap/README.md for more about that.
• x.py build --stage 1 builds the stage 1 compiler,
• x.py test --stage 1 src/tests/run-pass --test-args align-struct.rs builds stage 1 and runs matching tests.
• x.py test --stage 1 not that useful, some tests require a stage 2 compiler and will fail at stage 1.
• x.py test src/tools/tidy performs rust lint checks.
• On my machine it takes about an hour for a full stage 2 compiler build and test run.

The anatomy of the Rust compiler

• Rust consists of a number of core crates:
  • libsyntax contains things purely concerned with syntax, e.g. parser, AST, macro expander, etc.
  • librustc contains the high-level analysis passes, such as the type checker, borrow checker, etc. It is the heart of the compiler.
  • librustc_back contains some very low-level details that are specific to different LLVM targets.
  • librustc_trans contains the code to convert from Rust IR into LLVM IR, and then from LLVM IR into machine code.
  • librustc_driver invokes the compiler from libsyntax, then the analysis phases from librustc, and finally the lowering and codegen passes from librustc_trans.
• src/librustc/README.txt contains more high level documentation of the compiler.

Custom alignment & LLVM

• At some point I realise that just because mem::align_of and mem::size_of are reporting the right thing, it doesn't mean that everything is working.
• These have nothing to do with LLVM's picture of alignment.
• Seemingly no, you can't tell LLVM about custom alignment of types or fields.
• Needs to be managed from the Rust side.
• Need to emit alignment for alloca and manually pad fields and structs for custom alignment.
• I'm not going to get this done in my time off work!

An initial solution

• Rust keeps track of ADT layout in librustc ty::layout::Layout enum.
• Add a vector of custom alignment padding to this if [repr(align))] is used.
• Tell LLVM about custom padding fields and adjust various indices where required.
• Add a bunch of tests:
• src/test/run-pass - check what's supposed to work works
• src/test/compile-fail - check compile errors are handled correctly
• src/test/ui - print-type-sizes

22/02/2017

• The awesome EddyB from the core compiler team is assigned to review my PR.
• He makes a few of small suggestions to improve PR, improving naming of things and so on.
• The compiler now supports literal attributes:
  • so change [repr(align = "n")] to [repr(align(n))].
• I address all feedback and update PR.

25/02/2017

• After disucssions with the core compiler team, Eddyb suggests that instead of maintaining the custom padding vector that we always emit field padding, not just for custom alignment.
• This is a good suggestion, it means that repr align has the same code paths as normal code.
• That also means it impacts all of Rust, so it's a big change.
• Ultimately I'd made a big push to get the PR ready, but this meant I had a lot more work to do.

Let's talk about time

• I often wondered, where people find the time to do Open Source work?
• I have a full time job working on proprietary software.
• I only have a limited amount of time and energy outside of work.
• How do others balance this?
• What I did find is after making a large initial time investment, it got easier to make progress in small increments.
• I could do 30 mintues at breakfast or an hour or two in an evening and push things forward.
• Of course in Rust you are not alone, the core team and community will help when they can.
• You don't want things to drag on too long.
• Rust is always changing, the longer your PR takes the more conflicts you will need to resolve, which is more time.
• Some features are critical path, it might be blocking other work.
• You make a big time investment, you kind of want to see it get used.

Communication

• There's a number of different communication channels you can use:
  • IRC #rust-internals
  • Forums - https://internals.rust-lang.org/
  • Github - Issue tracker
  • Reddit - https://www.reddit.com/r/rust
• IRC is popular, but it was usually pretty dead during the hours I tended to work.
• Tried asking for help in the forums but it didn't yeild results in my case.
• Most of my communication was via the github issues related to my changes.
  • 51 comments on tracking issue
  • 151 comments on pull request!
  • Most of that is EddyB guiding me, thank you EddyB!
• Do whatever work for you.

Debugging the compiler

• After making this change it took a long time before the compiler would compile itself.
• This hadn't really been a probem before, but now I was making a significant change.
• Lots of code in librustc_trans used hard coded indices for accessing fields, these were no longer correct.
• What do you do when the compiler doesn't compile?
• Ask for help!
• Enable more debug info cp src/bootstrap/config.toml.example config.toml
• Can enable LLVM assertions/debuginfo, disable Rust optimization, etc.
• This can make compilation very slow!
• Other debugging tips:
  • export RUST_BACKTRACE=full
  • export RUST_LOG=rustc_trans=debug
  • sudo ~/.cargo/bin/rust-gdb -p <rustc pid>
  • break abort - breaks on LLVM assertion

A flurry of (in)activity

• 20/3/2017
  • A PR from EddyB that I'm waiting on gets merged to master.
  • At some point EddyB goes on vacation (I assume).
• 8/4/2017
  • I go on vacation.
• 12/4/2017
  • The struct field reordering change gets merged to master which breaks stage 1 compiler again.
  • I'm still on vacation.
  • Getting poked on github becase people are waiting on my PR :)
• 19/4/2017
  • Back from vacation, Eddyb spots the compiler breaking problem in my code, saving me a lot of debugging time!
• 21/4/2017
  • Final PR ready, all tests passing, PR accepted.
  • A test fails on ARM, don't have hardware but work out what it is and fix it.

25/4/2017

• Looking through the issue tracker, I notice a regression in Nightly Rust which sounds related to my change.
• Create a repro and fix the bug.
• Obviously this wasn't covered by Rust's test suite.
• EddyB makes me a minimal test case and I submit a fix.
• Possibly this wasn't the only thing I've broken out there in Rust land!

So what does it all mean?

This:

#![feature(attr_literals)]
#![feature(repr_align)]
#[repr(align(64))]
pub struct Align64(i32);
pub struct Nested64 {
    a: Align64,
    b: i32,
}
#[no_mangle]
pub fn nested64(a: Align64, b: i32) -> Nested64 {
    Nested64 { a, b }
}

Note: [repr(align)] is feature gated, I don't know what the processes or timeline for stabalisation is.

So what does it all mean?

Becomes this:

%Nested64 = type { %Align64, [0 x i8], i32, [60 x i8] }
%Align64 = type { i32, [60 x i8] }
; Function Attrs: nounwind uwtable
define void @nested64(
  %Nested64* noalias nocapture sret dereferenceable(128),
  %Align64* noalias nocapture readonly dereferenceable(64),
  i32) unnamed_addr #0 {
start:
  %3 = bitcast %Align64* %1 to i8*
  %4 = bitcast %Nested64* %0 to i8*
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull %4,
    i8* nonnull %3, i64 64, i32 64, i1 false)
  %5 = getelementptr inbounds %Nested64, %Nested64* %0, i64 0, i32 2
  store i32 %2, i32* %5, align 4
  ret void
}

Closing thoughts

• It was definitely rewarding getting my change in there.
• It was a lot more work than I anticipated;
  • Estimated 1 month, actual time more like 4 months (part time)!
  • Keep that in mind when chosing something to work on.
  • I was able to make the time, but what if I couldn't?
• Final PR was only +692 and -86 lines of code;
  • Working on large complex projects often doesn't involve writing a lot of code.
  • I think it's also important to spend some time working on my own Rust projects.
• I'd like to continue contributing when I have the time.
• I'm sure I won't underestimate what's involved next time! /s

Links

• RFC 1358 - https://github.com/rust-lang/rfcs/blob/master/text/1358-repr-align.md
• Tracking issue for RFC - https://github.com/rust-lang/rust/issues/33626
• Pull request - https://github.com/rust-lang/rust/pull/39999
• Merge commit - https://github.com/rust-lang/rust/commit/6d841da4a0d7629f826117f99052e3d4a7997a7e