Skip to content

Combining inline and target_feature attributes with recursive functions causing incorrect results. #53117

Description

@jackmott

add_stuff is a function with AVX2 simd intrinsics, set to inline always.
add_stuff_helper is a function with the target_feature set to enable AVX2 instructions.

If you run this in a release build, you will get an incorrect result, the return values from the recursive function do not add up. I believe this is because there is code being generated around the recursion that is not getting the avx2 target_feature applied.

If you make add_stuff not recursive, this all works fine.

The code below obviously does not make sense to use by itself, it works for instance if you put the target_feature on the add_stuff function directly, but this technique is useful for doing some nice SIMD metaprogramming with traits, and this bug makes that not work.

Is this a bug that can be fixed? Or an innate limitation of the inlining, target_features, and recursion? Is there a workaround?

#[cfg(target_arch = "x87")]
use std::arch::x86::*;
#[cfg(target_arch = "x86_64")]
use std::arch::x86_64::*;
use std::fmt::Debug;

#[inline(always)]
unsafe fn add_stuff(a: f32, count: i32) -> __m256 {
    let b = _mm256_set1_ps(2.0);
    let a2 = _mm256_set1_ps(a);
    if count < 4 {
        println!("count:{}",count);
        let r = _mm256_add_ps(_mm256_add_ps(a2, b), add_stuff(a, count + 1));
        println!("r:{:?}",r);
        r
    } else {
        _mm256_add_ps(a2, b)
    }
}

#[target_feature(enable = "avx2")]
unsafe fn add_stuff_helper() {
    let r = add_stuff(2.0,1);
    println!("raw avx {:?}",r);
}

fn main() {
    unsafe {
        add_stuff_helper();
    }
}

Environment:

This happens with rustc 1.27 stable through 1.31.0 nightly (at least)
All tested on linux, on a cpu that supports AVX2 instructions. Intel core i7 6700

Activity

  1. changed the title [-]Inconsistent results in Debug vs Release builds with recursive generic trait function[/-] [+]Combining inline and target_feature attributes with recursive functions causing incorrect results.[/+] on Aug 6, 2018
  2. jackmott commented on Aug 6, 2018

    @jackmott
    ContributorAuthor

    @alexcrichton could this be related to llvm bug here: https://bugs.llvm.org/show_bug.cgi?id=37358

  3. alexcrichton commented on Aug 6, 2018

    @alexcrichton
    Member

    Glancing at the IR, yeah, it looks like that bug unfortunately

  4. jackmott commented on Aug 6, 2018

    @jackmott
    ContributorAuthor

    any tricks one can use to work around that, until LLVM fixes it?

  5. alexcrichton commented on Aug 6, 2018

    @alexcrichton
    Member

    If add_stuff is annotated with #[target_feature(enable = "avx2")] it fixes the issue, but beyond that there's not a great workaround for this unfortunately. I only saw this at opt-level=3 so you can try opt-level=2, but that's not necessarily guaranteed to work.

  6. jackmott commented on Aug 6, 2018

    @jackmott
    ContributorAuthor

    Yeah for my use case I could just copypasta whenever I have a recursive simd function, which isn't often!
    How quick does llvm tend to fix issues like this? days, months, years? Any way for me to indicate interset in that thread?

  7. alexcrichton commented on Aug 6, 2018

    @alexcrichton
    Member

    In my experience so far LLVM bugs tend to fall in one of two buckets: fixed within a week or never fixed. Unfortunately that bug's been open for more than a week :(

    In that sense my historical experience is that it'll be awhile before the bug is fixed, but we haven't done much work on our part to try to bug those who may know how to fix it (or know who may be able to fix it, and we could always more aggressively do that!)

  8. jackmott commented on Aug 6, 2018

    @jackmott
    ContributorAuthor

    I'll see if I can get an account on the bugzilla and chime in.

  9. jackmott commented on Oct 22, 2018

    @jackmott
    ContributorAuthor

    Previous comments, now deleted, were using nightly 1 day too soon! #55073 fixes this issue, thanks @alexcrichton

  10. jackmott commented on Oct 23, 2018

    @jackmott
    ContributorAuthor

    reopening as the workaround was reverted. LLVM fix seems to be in the works so I will test when that lands.

    I think the fix in 55073 was causing some bugs anyway so glad to see LLVM fix is near.

  11. added
    A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.
    T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
    A-SIMDArea: SIMD (Single Instruction Multiple Data)
    on Jan 27, 2019
  12. steveklabnik commented on Feb 13, 2020

    @steveklabnik
    Contributor

    @jackmott did you ever get around to testing this again? :)

  13. jackmott commented on Feb 13, 2020

    @jackmott
    ContributorAuthor

    @steveklabnik yes, this is fixed now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.A-SIMDArea: SIMD (Single Instruction Multiple Data)T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions