Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Functionattrs incorrectly marks argument as readnone #19841

Closed Quuxplusone closed 10 years ago

Quuxplusone commented 10 years ago
Bugzilla Link PR19842
Status RESOLVED FIXED
Importance P normal
Reported by Robert Lougher (rob.lougher.llvm@gmail.com)
Reported on 2014-05-23 11:10:17 -0700
Last modified on 2014-06-16 08:20:21 -0700
Version trunk
Hardware PC All
CC greg.bedwell@sony.com, llvm-bugs@lists.llvm.org, nlewycky@google.com, paul_robinson@playstation.sony.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
Running the following IR through opt -functionattrs will incorrectly mark
%store's %p as readnone.  The bug is due to incorrectly handling the copy of
the pointer made by @get_pntr, because @get_pntr itself is marked readnone.

define i32* @get_pntr(i32* %p) nounwind uwtable {
entry:
  ret i32* %p
}
define void @store(i32* %p) noinline nounwind uwtable {
entry:
  %call = call i32* @get_pntr(i32* %p)
  store i32 10, i32* %call, align 4
  ret void
}

This can be seen with the following C testcase.  The bug leads to GVN
incorrectly removing the return load:

======== test.c ===========

int *get_pntr(int *p) {
    return p;
}

__attribute__((noinline))
void store(int *p) {
    int *p2 = get_pntr(p);
    *p2 = 10;
}

int test() {
    int i;
    store(&i);
    return i;
}
-----------------------------

Compile in two steps as follows:

clang -O1 -emit-llvm test.c -c -o test.bc
opt -basicaa -inline -functionattrs -gvn -S test.bc

We get the following IR:

--------------------------------------------------
define i32* @get_pntr(i32* readnone %p) {
entry:
  ret i32* %p
}

define void @store(i32* nocapture readnone %p) {
entry:
  store i32 10, i32* %p, align 4, !tbaa !1
  ret void
}

define i32 @test() {
entry:
  %i = alloca i32, align 4
  call void @store(i32* %i)
  ret i32 undef
}
--------------------------------------------------

GVN removes the load because it believes that the call to %store has no affect
on %i, and as %i has just been allocated, the loaded value is undefined.
Quuxplusone commented 10 years ago

Rob, was this fixed by r209870?

Quuxplusone commented 10 years ago
(In reply to comment #1)
> Rob, was this fixed by r209870?

Yes.  This can now be closed.