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

[Davide Italiano via llvm-commits]

On Wed, Feb 1, 2017 at 3:27 AM, Hal Finkel <hfinkel at anl.gov> wrote:
> 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?
>

We can probably have two tests. One, in tree now, testing the
combination naked + noinline.
The other, will just test a function that has only naked as atttribute.

Now on the other bug. That's not ideal, and I agree with should fix
it. I have an IR example (hope it makes sense):

```
define i32 @tinkywinky() #0 {
entry:
  ret i32 5
}

define i32 @patatino() {
entry:
  %call = call i32 @tinkywinky()
  %call1 = call i32 @dipsy(i32 %call)
  ret i32 %call1
}

declare i32 @dipsy(i32)

attributes #0 = { noinline }
```

I'm going to write a patch if you agree.

--
Davide

>
>
> 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
>



-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare