llvm dev - Oct 2017 - Bug in replaceUsesOfWith: does not keep addrspace consistent in GEP

Hello,

Calling `replaceUsesOfWith` with a value in a different addrspace does not
keep the addrspace of a GEP consistent. Is this known? Is this a bug or
expected behaviour?

Minimal counterexample link
<https://gist.github.com/bollu/152ba5e1c20c03c7fc6d8c7b23ba828f>

Reproduced here:

#include <iostream>
#include "llvm/ADT/APFloat.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Bitcode/BitcodeWriter.h"
#include "llvm/IR/BasicBlock.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/DerivedTypes.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/Instructions.h"
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/Type.h"
#include "llvm/IR/Verifier.h"
#include "llvm/Support/FileSystem.h"
using namespace llvm;
using namespace std;

static const bool NON_VAR_ARG = false;

static AssertingVH<Function> getOrCreateFunction(Module &m,
FunctionType *FTy,
                                                 std::string name) {
    Function *F = m.getFunction(name);
    if (F) return F;

    return Function::Create(FTy, GlobalValue::ExternalLinkage, name, &m);
};
static const bool SHOW_ERROR = false;

int main() {
    static LLVMContext ctx;
    static IRBuilder<> Builder(ctx);

    Module *m = new Module("Module", ctx);
    Function *F = getOrCreateFunction(
        *m,
        FunctionType::get(Builder.getInt64Ty(),
                          {PointerType::get(Builder.getInt32Ty(), 42),
                          PointerType::get(Builder.getInt32Ty(), 1)},
                          NON_VAR_ARG),
        "f");
    auto It = F->arg_begin();
    Value *Arg = &*It;
    It++;
    Value *Arg2 = &*It;

    BasicBlock *Entry = BasicBlock::Create(ctx, "entry", F);
    Builder.SetInsertPoint(Entry);

    Instruction *Slot = nullptr;*    if (SHOW_ERROR) {
        Slot = cast<Instruction>(Builder.CreateGEP(Arg,
{Builder.getInt64(1)}, "slot"));
        errs() << "Slot(original): " << *Slot <<
"\n";
        Slot->replaceUsesOfWith(Arg, Arg2);
        errs() << "Slot(replaced): " << *Slot <<
"\n";
    }
    else {
        Slot = cast<Instruction>(Builder.CreateGEP(Arg2,
{Builder.getInt64(1)}, "slot"));
    }
*    Value *TypedSlot = Builder.CreateBitCast(Slot,
PointerType::get(Builder.getInt64Ty(), 1), "slot_typed");
    Value *Load = Builder.CreateLoad(TypedSlot, "Val");
    Builder.CreateRet(Load);

    if (verifyModule(*m) == 1) {
        errs() << "module has an error: ";
        verifyModule(*m, &errs());
        report_fatal_error("buggy module.");
    }
    outs() << *m << "\n";
    // llvm::WriteBitcodeToFile(m, outs());

    return 1;
};



Output:

### `SHOW_ERROR = 0`
```ll
; ModuleID = 'Module'
source_filename = "Module"

define i64 @f(i32 addrspace(42)*, i32 addrspace(1)*) {
entry:
  %slot = getelementptr i32, i32 addrspace(1)* %1, i64 1
  %slot_typed = bitcast i32 addrspace(1)* %slot to i64 addrspace(1)*
  %Val = load i64, i64 addrspace(1)* %slot_typed
  ret i64 %Val
}
```

### `SHOW_ERROR = 1`
```
Slot(original):   %slot = getelementptr i32, i32 addrspace(42)* %0, i64 1
Slot(replaced):   %slot = getelementptr i32, i32 addrspace(1)* %1, i64
1*Assertion failed: (castIsValid(op, S, Ty) && "Invalid
cast!"),
function Create, file
/Users/bollu/work/LLVM-all/polly/llvm/lib/IR/Instructions.cpp, line
2592.
*[5]    56333 abort      ./prog
```


Thanks,

~Siddharth.

-- 
Sending this from my phone, please excuse any typos!
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171014/8fbb5d3d/attachment.html>

Hi Siddharth,

I haven’t read through your code or example, so my reply may be off but I would
say this is expected.
When you call replaceUsesOfWith, you have to make sure the values are
“compatible” before doing this.
If the address space of both values are different and that’s not okay, then you
will create wrong code.

Cheers,
-Quentin
> On Oct 13, 2017, at 5:17 PM, Siddharth Bhat via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Hello,
> 
> Calling `replaceUsesOfWith` with a value in a different addrspace does not
keep the addrspace of a GEP consistent. Is this known? Is this a bug or expected
behaviour?
> 
> Minimal counterexample link
<https://gist.github.com/bollu/152ba5e1c20c03c7fc6d8c7b23ba828f>
> 
> Reproduced here:
> 
> #include <iostream>
> #include "llvm/ADT/APFloat.h"
> #include "llvm/ADT/STLExtras.h"
> #include "llvm/ADT/SmallVector.h"
> #include "llvm/Bitcode/BitcodeWriter.h"
> #include "llvm/IR/BasicBlock.h"
> #include "llvm/IR/Constants.h"
> #include "llvm/IR/DerivedTypes.h"
> #include "llvm/IR/Function.h"
> #include "llvm/IR/IRBuilder.h"
> #include "llvm/IR/Instructions.h"
> #include "llvm/IR/LLVMContext.h"
> #include "llvm/IR/Module.h"
> #include "llvm/IR/Type.h"
> #include "llvm/IR/Verifier.h"
> #include "llvm/Support/FileSystem.h"
> using namespace llvm;
> using namespace std;
> 
> static const bool NON_VAR_ARG = false;
> 
> static AssertingVH<Function> getOrCreateFunction(Module &m,
FunctionType *FTy,
>                                                  std::string name) {
>     Function *F = m.getFunction(name);
>     if (F) return F;
> 
>     return Function::Create(FTy, GlobalValue::ExternalLinkage, name,
&m);
> };
> static const bool SHOW_ERROR = false;
> 
> int main() {
>     static LLVMContext ctx;
>     static IRBuilder<> Builder(ctx);
> 
>     Module *m = new Module("Module", ctx);
>     Function *F = getOrCreateFunction(
>         *m,
>         FunctionType::get(Builder.getInt64Ty(),
>                           {PointerType::get(Builder.getInt32Ty(), 42),
>                           PointerType::get(Builder.getInt32Ty(), 1)},
>                           NON_VAR_ARG),
>         "f");
>     auto It = F->arg_begin();
>     Value *Arg = &*It;
>     It++;
>     Value *Arg2 = &*It;
> 
>     BasicBlock *Entry = BasicBlock::Create(ctx, "entry", F);
>     Builder.SetInsertPoint(Entry);
> 
>     Instruction *Slot = nullptr;
>     if (SHOW_ERROR) {
>         Slot = cast<Instruction>(Builder.CreateGEP(Arg,
{Builder.getInt64(1)}, "slot"));
>         errs() << "Slot(original): " << *Slot
<< "\n";
>         Slot->replaceUsesOfWith(Arg, Arg2);
>         errs() << "Slot(replaced): " << *Slot
<< "\n";
>     }
>     else {
>         Slot = cast<Instruction>(Builder.CreateGEP(Arg2,
{Builder.getInt64(1)}, "slot"));
>     }
>     Value *TypedSlot = Builder.CreateBitCast(Slot,
PointerType::get(Builder.getInt64Ty(), 1), "slot_typed");
>     Value *Load = Builder.CreateLoad(TypedSlot, "Val");
>     Builder.CreateRet(Load);
> 
>     if (verifyModule(*m) == 1) {
>         errs() << "module has an error: ";
>         verifyModule(*m, &errs());
>         report_fatal_error("buggy module.");
>     }
>     outs() << *m << "\n";
>     // llvm::WriteBitcodeToFile(m, outs());
> 
>     return 1;
> };
> 
> 
> Output:
> ### `SHOW_ERROR = 0`
> ```ll
> ; ModuleID = 'Module'
> source_filename = "Module"
> 
> define i64 @f(i32 addrspace(42)*, i32 addrspace(1)*) {
> entry:
>   %slot = getelementptr i32, i32 addrspace(1)* %1, i64 1
>   %slot_typed = bitcast i32 addrspace(1)* %slot to i64 addrspace(1)*
>   %Val = load i64, i64 addrspace(1)* %slot_typed
>   ret i64 %Val
> }
> ```
> 
> ### `SHOW_ERROR = 1`
> ```
> Slot(original):   %slot = getelementptr i32, i32 addrspace(42)* %0, i64 1
> Slot(replaced):   %slot = getelementptr i32, i32 addrspace(1)* %1, i64 1
> Assertion failed: (castIsValid(op, S, Ty) && "Invalid
cast!"), function Create, file
/Users/bollu/work/LLVM-all/polly/llvm/lib/IR/Instructions.cpp, line 2592.
> [5]    56333 abort      ./prog
> ```
> 
> Thanks,
> ~Siddharth.
> -- 
> Sending this from my phone, please excuse any typos!
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171013/c87a26c6/attachment.html>

I see. So, compatibility of two values depends on address spaces as well? I
didn't know that, thank you.

In that case, is there some uniform way to say "give me a new instruction
with this (possibly incompatible) value"?

For context, I'm taking some code that was generated in a different address
space context, and I'm copying it to a different context (with a different
address space).

Thanks,
Siddharth.

On Sat 14 Oct, 2017, 02:30 Quentin Colombet, <qcolombet at apple.com>
wrote:
> Hi Siddharth,
>
> I haven’t read through your code or example, so my reply may be off but I
> would say this is expected.
> When you call replaceUsesOfWith, you have to make sure the values are
> “compatible” before doing this.
> If the address space of both values are different and that’s not okay,
> then you will create wrong code.
>
> Cheers,
> -Quentin
>
> On Oct 13, 2017, at 5:17 PM, Siddharth Bhat via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> Hello,
>
> Calling `replaceUsesOfWith` with a value in a different addrspace does not
> keep the addrspace of a GEP consistent. Is this known? Is this a bug or
> expected behaviour?
>
> Minimal counterexample link
> <https://gist.github.com/bollu/152ba5e1c20c03c7fc6d8c7b23ba828f>
>
> Reproduced here:
>
> #include <iostream>
> #include "llvm/ADT/APFloat.h"
> #include "llvm/ADT/STLExtras.h"
> #include "llvm/ADT/SmallVector.h"
> #include "llvm/Bitcode/BitcodeWriter.h"
> #include "llvm/IR/BasicBlock.h"
> #include "llvm/IR/Constants.h"
> #include "llvm/IR/DerivedTypes.h"
> #include "llvm/IR/Function.h"
> #include "llvm/IR/IRBuilder.h"
> #include "llvm/IR/Instructions.h"
> #include "llvm/IR/LLVMContext.h"
> #include "llvm/IR/Module.h"
> #include "llvm/IR/Type.h"
> #include "llvm/IR/Verifier.h"
> #include "llvm/Support/FileSystem.h"
> using namespace llvm;
> using namespace std;
>
> static const bool NON_VAR_ARG = false;
>
> static AssertingVH<Function> getOrCreateFunction(Module &m,
FunctionType *FTy,
>                                                  std::string name) {
>     Function *F = m.getFunction(name);
>     if (F) return F;
>
>     return Function::Create(FTy, GlobalValue::ExternalLinkage, name,
&m);
> };
> static const bool SHOW_ERROR = false;
>
> int main() {
>     static LLVMContext ctx;
>     static IRBuilder<> Builder(ctx);
>
>     Module *m = new Module("Module", ctx);
>     Function *F = getOrCreateFunction(
>         *m,
>         FunctionType::get(Builder.getInt64Ty(),
>                           {PointerType::get(Builder.getInt32Ty(), 42),
>                           PointerType::get(Builder.getInt32Ty(), 1)},
>                           NON_VAR_ARG),
>         "f");
>     auto It = F->arg_begin();
>     Value *Arg = &*It;
>     It++;
>     Value *Arg2 = &*It;
>
>     BasicBlock *Entry = BasicBlock::Create(ctx, "entry", F);
>     Builder.SetInsertPoint(Entry);
>
>     Instruction *Slot = nullptr;*    if (SHOW_ERROR) {
>         Slot = cast<Instruction>(Builder.CreateGEP(Arg,
{Builder.getInt64(1)}, "slot"));
>         errs() << "Slot(original): " << *Slot
<< "\n";
>         Slot->replaceUsesOfWith(Arg, Arg2);
>         errs() << "Slot(replaced): " << *Slot
<< "\n";
>     }
>     else {
>         Slot = cast<Instruction>(Builder.CreateGEP(Arg2,
{Builder.getInt64(1)}, "slot"));
>     }
> *    Value *TypedSlot = Builder.CreateBitCast(Slot,
PointerType::get(Builder.getInt64Ty(), 1), "slot_typed");
>     Value *Load = Builder.CreateLoad(TypedSlot, "Val");
>     Builder.CreateRet(Load);
>
>     if (verifyModule(*m) == 1) {
>         errs() << "module has an error: ";
>         verifyModule(*m, &errs());
>         report_fatal_error("buggy module.");
>     }
>     outs() << *m << "\n";
>     // llvm::WriteBitcodeToFile(m, outs());
>
>     return 1;
> };
>
>
>
> Output:
>
> ### `SHOW_ERROR = 0`
> ```ll
> ; ModuleID = 'Module'
> source_filename = "Module"
>
> define i64 @f(i32 addrspace(42)*, i32 addrspace(1)*) {
> entry:
>   %slot = getelementptr i32, i32 addrspace(1)* %1, i64 1
>   %slot_typed = bitcast i32 addrspace(1)* %slot to i64 addrspace(1)*
>   %Val = load i64, i64 addrspace(1)* %slot_typed
>   ret i64 %Val
> }
> ```
>
> ### `SHOW_ERROR = 1`
> ```
> Slot(original):   %slot = getelementptr i32, i32 addrspace(42)* %0, i64 1
> Slot(replaced):   %slot = getelementptr i32, i32 addrspace(1)* %1, i64
1*Assertion failed: (castIsValid(op, S, Ty) && "Invalid
cast!"), function Create, file
/Users/bollu/work/LLVM-all/polly/llvm/lib/IR/Instructions.cpp, line 2592.
> *[5]    56333 abort      ./prog
> ```
>
>
> Thanks,
>
> ~Siddharth.
>
> --
> Sending this from my phone, please excuse any typos!
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
> --
Sending this from my phone, please excuse any typos!
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171014/f952a373/attachment.html>