Merge pull request #666 from FractalFir/master

Fixed a recursive inling bug, added a test for it

Author: antoyo

src/attributes.rs

@@ -6,21 +6,68 @@ use rustc_attr_parsing::InlineAttr;
 use rustc_attr_parsing::InstructionSetAttr;
 #[cfg(feature = "master")]
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
+use rustc_middle::mir::TerminatorKind;
 use rustc_middle::ty;
 
 use crate::context::CodegenCx;
 use crate::gcc_util::to_gcc_features;
 
-/// Get GCC attribute for the provided inline heuristic.
+/// Checks if the function `instance` is recursively inline.
+/// Returns `false` if a functions is guaranteed to be non-recursive, and `true` if it *might* be recursive.
+#[cfg(feature = "master")]
+fn resursively_inline<'gcc, 'tcx>(
+    cx: &CodegenCx<'gcc, 'tcx>,
+    instance: ty::Instance<'tcx>,
+) -> bool {
+    // No body, so we can't check if this is recursively inline, so we assume it is.
+    if !cx.tcx.is_mir_available(instance.def_id()) {
+        return true;
+    }
+    // `expect_local` ought to never fail: we should be checking a function within this codegen unit.
+    let body = cx.tcx.optimized_mir(instance.def_id());
+    for block in body.basic_blocks.iter() {
+        let Some(ref terminator) = block.terminator else { continue };
+        // I assume that the recursive-inline issue applies only to functions, and not to drops.
+        // In principle, a recursive, `#[inline(always)]` drop could(?) exist, but I don't think it does.
+        let TerminatorKind::Call { ref func, .. } = terminator.kind else { continue };
+        let Some((def, _args)) = func.const_fn_def() else { continue };
+        // Check if the called function is recursively inline.
+        if matches!(
+            cx.tcx.codegen_fn_attrs(def).inline,
+            InlineAttr::Always | InlineAttr::Force { .. }
+        ) {
+            return true;
+        }
+    }
+    false
+}
+
+/// Get GCC attribute for the provided inline heuristic, attached to `instance`.
 #[cfg(feature = "master")]
 #[inline]
 fn inline_attr<'gcc, 'tcx>(
     cx: &CodegenCx<'gcc, 'tcx>,
     inline: InlineAttr,
+    instance: ty::Instance<'tcx>,
 ) -> Option<FnAttribute<'gcc>> {
     match inline {
+        InlineAttr::Always => {
+            // We can't simply always return `always_inline` unconditionally.
+            // It is *NOT A HINT* and does not work for recursive functions.
+            //
+            // So, it can only be applied *if*:
+            // The current function does not call any functions marked `#[inline(always)]`.
+            //
+            // That prevents issues steming from recursive `#[inline(always)]` at a *relatively* small cost.
+            // We *only* need to check all the terminators of a function marked with this attribute.
+            if resursively_inline(cx, instance) {
+                Some(FnAttribute::Inline)
+            } else {
+                Some(FnAttribute::AlwaysInline)
+            }
+        }
         InlineAttr::Hint => Some(FnAttribute::Inline),
-        InlineAttr::Always | InlineAttr::Force { .. } => Some(FnAttribute::AlwaysInline),
+        InlineAttr::Force { .. } => Some(FnAttribute::AlwaysInline),
         InlineAttr::Never => {
             if cx.sess().target.arch != "amdgpu" {
                 Some(FnAttribute::NoInline)
@@ -52,7 +99,7 @@ pub fn from_fn_attrs<'gcc, 'tcx>(
         } else {
             codegen_fn_attrs.inline
         };
-        if let Some(attr) = inline_attr(cx, inline) {
+        if let Some(attr) = inline_attr(cx, inline, instance) {
             if let FnAttribute::AlwaysInline = attr {
                 func.add_attribute(FnAttribute::Inline);
             }

tests/run/always_inline.rs

@@ -0,0 +1,53 @@
+// Compiler:
+//
+// Run-time:
+//   status: 0
+
+#![feature(no_core)]
+#![no_std]
+#![no_core]
+#![no_main]
+
+extern crate mini_core;
+use mini_core::*;
+
+#[inline(always)]
+fn fib(n: u8) -> u8 {
+    if n == 0 {
+        return 1;
+    }
+    if n == 1 {
+        return 1;
+    }
+    fib(n - 1) + fib(n - 2)
+}
+
+#[inline(always)]
+fn fib_b(n: u8) -> u8 {
+    if n == 0 {
+        return 1;
+    }
+    if n == 1 {
+        return 1;
+    }
+    fib_a(n - 1) + fib_a(n - 2)
+}
+
+#[inline(always)]
+fn fib_a(n: u8) -> u8 {
+    if n == 0 {
+        return 1;
+    }
+    if n == 1 {
+        return 1;
+    }
+    fib_b(n - 1) + fib_b(n - 2)
+}
+
+#[no_mangle]
+extern "C" fn main(argc: i32, _argv: *const *const u8) -> i32 {
+    if fib(2) != fib_a(2) {
+        intrinsics::abort();
+    }
+    0
+}