[llvm-commits] [llvm] r127565 - /llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

[Duncan Sands]

Hi Jin-Gu,

> @@ -433,6 +433,18 @@
>             !SI.isVolatile()) {
>           if (LI == Val)
>             return EraseInstFromFunction(SI);
> +        // load and store reference same memory location, the memory location

load -> Load
> +        // is represented by getelementptr with two uses (load and store) and
> +        // the getelementptr's base is alloca with single use. At this point,
> +        // instructions from alloca to store can be removed.
> +        // (this pattern is generated when bitfield is accessed.)
(this pattern is generated when bitfield is accessed.) -> This can happen when 
accessing bitfields.

> +        // For example,
> +        // %u = alloca %struct.test, align 4               ; [#uses=1]
> +        // %0 = getelementptr inbounds %struct.test* %u, i32 0, i32 0;[#uses=2]
> +        // %1 = load i8* %0, align 4                       ; [#uses=1]
> +        // %2 = and i8 %1, -16                             ; [#uses=1]
> +        // %3 = or i8 %2, 5                                ; [#uses=1]
> +        // store i8 %3, i8* %0, align 4

The rest of the comment shouldn't be here.

>           if (Ptr->hasNUses(2)) {

As I already mentioned this test does not test what the comment says you are
testing, and what you need to test.

>             if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(Ptr)) {
>               if (isa<AllocaInst>(GEP->getOperand(0))) {

As a meta-question: does the dse pass not handle this case?  If not, that should
probably be analysed and fixed.

Ciao, Duncan.