[PATCH] D36233: [ThinLTO] Add FunctionAttrs to ThinLTO index

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: test/Bitcode/thinlto-function-summary-functionattrs.ll:4
+
+; CHECK: <GLOBALVAL_SUMMARY_BLOCK
+; ensure @f is marked readnone
----------------
tejohnson wrote:
> tejohnson wrote:
> > ncharlie wrote:
> > > tejohnson wrote:
> > > > ncharlie wrote:
> > > > > tejohnson wrote:
> > > > > > Can you add a check for noalias as well?
> > > > > Sure - is there some trick to force a function to be marked as noalias?
> > > > > 
> > > > > I can't find any examples of noalias functions in any of the tests and this doesn't seem to work:
> > > > > ```
> > > > > define noalias i32* @i() {
> > > > >    %d = alloca i32;
> > > > >    ret i32* %d;
> > > > > }
> > > > > ```
> > > > I see a few in the tests, here are some samples:
> > > > test/Transforms/SimplifyCFG/2011-03-08-UnreachableUse.ll:define noalias i8* @func_29() nounwind {
> > > > test/Transforms/GVN/malloc-load-removal.ll:define noalias i8* @test1() nounwind uwtable ssp {
> > > > test/Transforms/NewGVN/malloc-load-removal.ll:define noalias i8* @test1() nounwind uwtable ssp {
> > > > test/Transforms/LoopIdiom/basic.ll:define noalias i32* @test17(i32* nocapture readonly %a, i32 %c) {
> > > > test/Transforms/DeadStoreElimination/simple.ll:define noalias i8* @test23() nounwind uwtable ssp {
> > > > test/Transforms/FunctionAttrs/nonnull.ll:; CHECK: define noalias nonnull i8* @test4_helper
> > > > 
> > > > In what way does it not work when you try it for your small example?
> > > > 
> > > The functions (even with the noalias keyword) don't have the NoAlias flag set on them in my example or in the tests.
> > Somehow it is getting propagated. I.e. I took one of these tests (DeadStoreElimination/simple.ll) and did the following
> > 
> > $ llvm-as test/Transforms/DeadStoreElimination/simple.ll -o - | llvm-dis -o - | grep test23
> > 
> > I get:
> > define noalias i8* @test23() #3 {
> > 
> > Can you check this example?
> To save you some work, since I (sort of) see what is going on by looking in the debugger:
> 
> (gdb) p F.dump()
> 
> ; Function Attrs: nounwind ssp uwtable
> define noalias i8* @test23() #3 {
>   %x = alloca [2 x i8], align 1
>   %arrayidx = getelementptr inbounds [2 x i8], [2 x i8]* %x, i64 0, i64 0
>   store i8 97, i8* %arrayidx, align 1
>   %arrayidx1 = getelementptr inbounds [2 x i8], [2 x i8]* %x, i64 0, i64 1
>   store i8 0, i8* %arrayidx1, align 1
>   %call = call i8* @strdup(i8* %arrayidx) #1
>   ret i8* %call
> }
> $42 = void
> (gdb) p F.hasFnAttribute(Attribute::NoAlias)
> $43 = false
> (gdb) p F.returnDoesNotAlias()
> $44 = true
> 
> Note the function is dumped with "noalias", but doesn't have the NoAlias attribute. However, the return is marked no alias. And looking at the FunctionAttrs.cpp NoAlias propagation (addNoAliasAttrs), it is this (the returnDoesNotAlias) that is being propagated. So perhaps that's what you need to be checking. Probably change the name of the boolean flag in the index accordingly.
And just to close the loop on this, looking at AssemblyWriter::printFunction, the ReturnIndex attributes are printed before the function name (which matches where we see "noalias" on functions), whereas the FunctionIndex attributes are printed after the function name (where you see readonly, etc being printed).


https://reviews.llvm.org/D36233