Page MenuHomePhabricator

clang appears not to respect __attribute__((noinline))
Needs RevisionPublic

Authored by fdeferriere on Oct 19 2018, 4:28 AM.

Details

Summary

Bug 26545 reports that the "noinline" attribute does not prevent the result of a function to be "inlined" into the caller.
The problem comes in fact from the Inter-Procedural Sparse Constant Propagation which propagates the return value into the caller.
This patch fixes this problem by patching the function "canTrackReturnsInterprocedurally". This function now returns false on functions with the "noinline" attribute.

Test case:
llvm/test/Transforms/IPConstantProp/noinline.ll

; RUN: opt -ipsccp -S %s | FileCheck %s

define internal fastcc i32 @v1() #0 {
entry:

ret i32 1

; No IPSCCP when noinline attribute
; CHECK: ret i32 1
}

define internal fastcc i32 @v2() {
entry:

ret i32 2

; IPSCCP on this function
; CHECK: ret i32 undef
}

define internal fastcc i32 @foo(i32 %x, i32 %y) {
entry:

%add = add nsw i32 %y, %x
%call = tail call fastcc i32 @v1()
%add1 = add nsw i32 %add, %call
%call2 = tail call fastcc i32 @v2()
%add3 = add nsw i32 %add1, %call2
ret i32 %add3

; CHECK: ret i32 %add3
}

define i32 @bar() {
entry:

%call = tail call fastcc i32 @v1()
%call1 = tail call fastcc i32 @v2()
%call2 = tail call fastcc i32 @foo(i32 %call, i32 %call1)
ret i32 %call2

; CHECK ret i32 %call2
}

attributes #0 = { noinline }

Diff Detail

Event Timeline

fdeferriere created this revision.Oct 19 2018, 4:28 AM

Test?
Also, please upload all patches with full context (-U99999)

fdeferriere removed a project: Restricted Project.Oct 19 2018, 4:43 AM

git diff -U99999

fdeferriere edited the summary of this revision. (Show Details)Oct 22 2018, 2:01 AM
chandlerc requested changes to this revision.Oct 22 2018, 1:09 PM

The intent of noinline in LLVM's IR is to block inlining, not all interprocedural optimizations. So I don't think this is actually a bug and don't think this patch should be applied.

We have an attribute to block optimizations: optnone.

This revision now requires changes to proceed.Oct 22 2018, 1:09 PM

The intent of noinline in LLVM's IR is to block inlining, not all interprocedural optimizations. So I don't think this is actually a bug

While this position may be correct in the technical sense, I don't think it's helpful to real-world developers. To a non-compiler-engineer, what's happening is "inlining", and clang is doing an unexpected thing by dropping the function call.

When people in my organization ask me why this doesn't work, I don't want to have to explain "it's not actually inlining" nor "use optnone". It would be great if there could be a change in the behavior here (but I have no expertise on whether this specific patch is the right way to achieve it).

chrib added a subscriber: chrib.Dec 4 2018, 8:40 AM
chrib added a comment.Dec 5 2018, 12:48 AM

actually optnone is not well adapted here, since it demotes local optimizations on the functions, but we still want to keep scalar or target optimisations to be applied, but not interprocedural optimisations.

so maybe a new attribute , e.g ((noglobalopt) would be useful to exclude an object from all global optimizations (dfe, dve, global constant prop, ...). Do you think it is worth to make a proposal ?

actually optnone is not well adapted here, since it demotes local optimizations on the functions, but we still want to keep scalar or target optimisations to be applied, but not interprocedural optimisations.

so maybe a new attribute , e.g ((noglobalopt) would be useful to exclude an object from all global optimizations (dfe, dve, global constant prop, ...). Do you think it is worth to make a proposal ?

I do not think we can realistically support slicing and dicing optimizations in this way. We allow turning them off to help bisect problems and work around them, but IMO this is going too far.