Skip to content
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

make use of LLVM's scoped noalias metadata #16515

Open
thestinger opened this Issue Aug 15, 2014 · 17 comments

Comments

Projects
None yet
@thestinger
Copy link
Contributor

thestinger commented Aug 15, 2014

Rust could pass along lots of aliasing information to LLVM via the new metadata.

http://llvm.org/docs/LangRef.html#noalias-and-alias-scope-metadata


UPDATE(@eddyb): Currently blocked on an Unsafe Code Guidelines decision, regarding the exact cases we can treat as UB, and optimize - see #16515 (comment) below, and further comments.

While LLVM can and will convert noalias attributes in function signatures into scoped !noalias metadata on instructions, when inlining, we never emit any such metadata ourselves.

Furthermore, the LLVM conversion is conservative (i.e. scoped to the entire callee) in a way that can't easily be generalized intra-function, without deciding on what to define and what to leave as UB.

@luqmana

This comment has been minimized.

Copy link
Member

luqmana commented Aug 22, 2014

There's already an LLVM pass which converts the noalias attributes to the new metadata during inlining. It's just not enabled by default now, but you can try it out by passing -C llvm-args="-enable-noalias-to-md-conversion".

@thestinger

This comment has been minimized.

Copy link
Contributor Author

thestinger commented Aug 22, 2014

I'm aware of that, but we have lots of aliasing information beyond just the outermost pointer in a parameter.

@steveklabnik steveklabnik added the A-LLVM label Feb 11, 2015

@cmr cmr self-assigned this Mar 25, 2015

@mahkoh

This comment has been minimized.

Copy link
Contributor

mahkoh commented Nov 19, 2015

In the following code, one would expect b, c, and d to generate the same code. Instead, a and c generate the same code because the aliasing guarantees of & are not used in c.

#![crate_type = "lib"]

pub unsafe fn a(p: *const usize, g: fn()) -> usize {
    let a = *p;
    g();
    let b = *p;
    a ^ b
}

pub unsafe fn b(p: &usize, g: fn()) -> usize {
    let a = *p;
    g();
    let b = *p;
    a ^ b
}

pub unsafe fn c(p: *const usize, g: fn()) -> usize {
    let p = &*p;

    let a = *p;
    g();
    let b = *p;
    a ^ b
}

pub unsafe fn d(p: *const usize, g: fn()) -> usize {
    let p = &*p;

    b(p, g)
}

Applying the information suggested in this issue manually to the IR fixes the issue.

b:
    pushq   %rax
    callq   *%rsi
    xorl    %eax, %eax
    popq    %rdx
    retq

c:
    pushq   %r14
    pushq   %rbx
    pushq   %rax
    movq    %rdi, %r14
    movq    (%r14), %rbx
    callq   *%rsi
    xorq    (%r14), %rbx
    movq    %rbx, %rax
    addq    $8, %rsp
    popq    %rbx
    popq    %r14
    retq
@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Sep 30, 2017

Triage: #38941 is related, as is #31681

@varkor

This comment has been minimized.

Copy link
Member

varkor commented Dec 12, 2017

@mahkoh: I'm interested in looking into this — could you provide any more details on what metadata you added to successfully reduce the code generated for c to that for b and d (and how you tested it)?

@varkor

This comment has been minimized.

Copy link
Member

varkor commented Dec 12, 2017

Following up on this particular test case, the optimised (opt-level=3) LLVM IR for the bodies of both a and c is:

; a, c
%0 = load i64, i64* %p, align 8
tail call void %g()
%1 = load i64, i64* %p, align 8
%2 = xor i64 %1, %0
ret i64 %2

whereas the ideal IR for c (already generated for b and d) is:

; b, d
tail call void %g()
ret i64 0

If we manually annotate the IR, so that the body of c is:

; c
%0 = load i64, i64* %p, align 8, !alias.scope !0
tail call void %g(), !noalias !0
%1 = load i64, i64* %p, align 8, !alias.scope !0
%2 = xor i64 %1, %0
ret i64 %2

and we add the following domain and scope:

!2 = distinct !{!2} ; domain
!1 = distinct !{!1, !2} ; scope
!0 = distinct !{!1} ; scope list

then, after running LLVM's opt -O3, the body of c is reduced to:

; c
tail call void %g(), !noalias !0
ret i64 0

as we want.

@varkor

This comment has been minimized.

Copy link
Member

varkor commented Dec 21, 2017

Update: the semantics for aliasing guarantees have not by this time been completely well defined — in particular the above transformation is guaranteed (at least now) to be correct. It's probably necessary to wait before implementing any features based on alias.scope or noalias because of this.

@varkor

This comment has been minimized.

Copy link
Member

varkor commented Jan 10, 2018

Just in case this is ever useful in the future, I have a branch which optimises the test case using the metadata. Even though it may not be semantically correct, it might be helpful to look at for other alias optimisations in the future.

@nox

This comment has been minimized.

Copy link
Contributor

nox commented Mar 31, 2018

Cc @rust-lang/wg-codegen @rust-lang/compiler This looks like it should be carried to completion by one of us, after @varkor's concern about it being semantically correct or not is addressed.

@varkor

This comment has been minimized.

Copy link
Member

varkor commented Mar 31, 2018

@nox: in particular, we need some examples of test cases that are improved by some alias.scope annotations, which are also sound. I think after we have some concrete examples, it shouldn't prove too difficult to adapt the previous work.

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Apr 1, 2018

IIRC this is blocked on the unsafe code guidelines folks figuring out where aliasing UB starts? cc @RalfJung

@RalfJung

This comment has been minimized.

Copy link
Member

RalfJung commented Apr 1, 2018

Sounds reasonable. ;) I'd be hesitant to commit to anything right now though. However, I am fairly close to having a model that (I think) determines some kind of "lower bound" (in the sense of "we want at least this much UB"), so I could try to understand if the examples you have here work under that model. However, longer-term, it'd also be useful for me to actually understand what LLVM's scoped metadata means in general, and how you plan to emit it, so that we can try to get certainty not just for some examples.

So, looking at #16515 (comment), we seem to be in agreement that a cannot be optimized. b should be optimizable under any model we come up with, I hope. Optimizing c is an explicit goal of the "lower bound" model I have been toying around with at the all-hands (and that I plan to work on more during the summer). d is almost equivalent to c (though it doesn't seem unlikely that function boundaries will have some significance), but with b optimizable the same of course holds for d.

@varkor did you mean to write c at the bottom of #16515 (comment)? I thought b is already optimized right now, without additional metadata?

So, the only "new" example here (that would get optimized now but was not optimized before) is c? I think we want that to happen. And I think for shared references, this should not be too hard. Not sure how important it is that we proceed here?

But please hold on with anything related to mutable references. There are some strange corner cases where optimizing within a function is illegal but after moving some code to a separate function it becomes legal:

fn legal(x: &mut i32) { unsafe {
  let x = x as *mut _ as usize;
  let y1 = &mut *(x as *mut i32);
  let y2 = &mut *(y1 as *mut i32 as usize as *mut i32);
  // these two writes are legal -- y2 is reborrowing from y1, but because 
  // we went though usize we cannot possibly track where they are "derived from"
  *y2 = 4;
  *y1 = 3;
  // Using y2 again here would be UB
} }

fn illegal(x: &mut i32) { unsafe {
  let x = x as *mut _ as usize;
  let y1 = &mut *(x as *mut i32);
  let y2 = &mut *(y1 as *mut i32 as usize as *mut i32);

  bar(y1, y2); // this call must trigger UB
} }
fn bar(y1: &mut i32, y2: &mut i32) {
  // we will certainly want to optimize this
  *y2 = 4;
  *y1 = 3;
}

illegal and legal look like they do the same thing, but we have to rule out illegal and most likely have to allow legal. We want to understand this better before we start optimizing more.

@varkor

This comment has been minimized.

Copy link
Member

varkor commented Apr 1, 2018

@varkor did you mean to write c at the bottom of #16515 (comment)? I thought b is already optimized right now, without additional metadata?

Yes, I did, thanks — I've fixed the comment.

@RalfJung

This comment has been minimized.

Copy link
Member

RalfJung commented Dec 10, 2018

It seems that besides the noalias attribute (that Rust already uses) and the noalias metadata (that this issue proposes to use), there is the llvm.noalias intrinsic. And judging from the PR introducing that intrinsic, the metadata has some issues that are solved by the intrinsic.

This issue is older than the intrinsic. With the intrinsic now being available, do y'all think that the intrinsic should be better suited than the metadata for Rust's purpose, or is the metadata still preferred?

(One of the reasons I am asking is that I hope to start looking at LLVM's noalias stuff from a formal perspective, and it'd be helpful if we didn't have to look at three different mechanisms to do very similar things.)

@nikic

This comment has been minimized.

Copy link
Contributor

nikic commented Dec 10, 2018

@RalfJung The llvm.noalias intrinsic never landed. In https://bugs.llvm.org/show_bug.cgi?id=39282 Hal mentioned that somebody is working on this area again, but I haven't heard anything since.

@RalfJung

This comment has been minimized.

Copy link
Member

RalfJung commented Dec 10, 2018

The llvm.noalias intrinsic never landed.

Oh, seems I misinterpreted the "Accepted" label then.

In that case, take my question as the hypothetical "if those patches landed, would Rust prefer that over the metadata?"^^

@RalfJung

This comment has been minimized.

Copy link
Member

RalfJung commented Mar 25, 2019

An RFC has been posted on the LLVM side for extending the support for restrict/noalias: https://lists.llvm.org/pipermail/llvm-dev/2019-March/131127.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.