[llvm] r293727 - [IPSCCP] Teach how to not propagate return values of naked functions.

[Hal Finkel via llvm-commits]

Hi Davide,

This reminds me that we've have a long-standing problem with this an 
noinline in general. Specifically,

int foo { return 5; } __attribute__((noinline))

will essentially still be inlined because of return-value propagation. I 
think that we should fix that too. Regardless, can we remove noinline 
from the test case for naked so that we make sure we continue to test 
the naked handling? Or, if naked functions must be noinline, can we 
generalize this to just handling noinline functions instead?

Thanks again,

Hal


On 01/31/2017 07:01 PM, Davide Italiano via llvm-commits wrote:
> Author: davide
> Date: Tue Jan 31 19:01:22 2017
> New Revision: 293727
>
> URL: http://llvm.org/viewvc/llvm-project?rev=293727&view=rev
> Log:
> [IPSCCP] Teach how to not propagate return values of naked functions.
>
> Differential Revision:  https://reviews.llvm.org/D29360
>
> Added:
>      llvm/trunk/test/Transforms/IPConstantProp/naked-return.ll
> Modified:
>      llvm/trunk/lib/Transforms/Scalar/SCCP.cpp
>
> Modified: llvm/trunk/lib/Transforms/Scalar/SCCP.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SCCP.cpp?rev=293727&r1=293726&r2=293727&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/SCCP.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/SCCP.cpp Tue Jan 31 19:01:22 2017
> @@ -1712,7 +1712,10 @@ static bool runIPSCCP(Module &M, const D
>   
>       // If this is an exact definition of this function, then we can propagate
>       // information about its result into callsites of it.
> -    if (F.hasExactDefinition())
> +    // Don't touch naked functions. They may contain asm returning a
> +    // value we don't see, so we may end up interprocedurally propagating
> +    // the return value incorrectly.
> +    if (F.hasExactDefinition() && !F.hasFnAttribute(Attribute::Naked))
>         Solver.AddTrackedFunction(&F);
>   
>       // If this function only has direct calls that we can see, we can track its
>
> Added: llvm/trunk/test/Transforms/IPConstantProp/naked-return.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IPConstantProp/naked-return.ll?rev=293727&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/IPConstantProp/naked-return.ll (added)
> +++ llvm/trunk/test/Transforms/IPConstantProp/naked-return.ll Tue Jan 31 19:01:22 2017
> @@ -0,0 +1,28 @@
> +; RUN: opt -ipsccp -S %s | FileCheck %s
> +
> +target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
> +target triple = "i686-pc-windows-msvc19.0.24215"
> +
> +define i32 @dipsy(i32, i32) local_unnamed_addr #0 {
> +BasicBlock0:
> +  call void asm "\0D\0Apushl %ebp\0D\0Amovl 8(%esp),%eax\0D\0Amovl 12(%esp), %ebp\0D\0Acalll *%eax\0D\0Apopl %ebp\0D\0Aretl\0D\0A", ""()
> +  ret i32 0
> +}
> +
> +define void @tinkywinky(i32, i32, i32) local_unnamed_addr #0 {
> +BasicBlock1:
> +  call void asm "\0D\0A    movl 12(%esp), %ebp\0D\0A    movl 4(%esp), %eax\0D\0A    movl 8(%esp), %esp\0D\0A    jmpl *%eax\0D\0A", ""()
> +  ret void
> +}
> +
> +define void @patatino(i32, i32, i32) local_unnamed_addr #1 {
> +bb:
> +  %3 = tail call i32 @dipsy(i32 %0, i32 %1) #0
> +; Check that we don't accidentally propagate zero.
> +; CHECK: @tinkywinky(i32 %3, i32 %2, i32 %1) #0
> +  tail call void @tinkywinky(i32 %3, i32 %2, i32 %1) #0
> +  ret void
> +}
> +
> +attributes #0 = { naked noinline optnone }
> +attributes #1 = { "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory