Op's create methods support passing an instruction name

[JanRehders-AMD]

Equivalent to CallInst, for more readable code. This allows to pass in a value name without a call to setName which is especially useful when directly returning a create op.

For example:

return m_builder->create<ExampleOp>(m_builder->getInt32(123), "example_123");

[JanRehders-AMD]

@nhaehnle @Flakebi could you add yourself as reviewers? Didn't do that when creating the PR and don't have enough rights to add you later

[tsymalla]

You need to update the generated files for the Example dialect for this to pass CI.

[tsymalla-AMD]

Can you please do the following:

• Update the tests
• Show how this plays with an op argument called instName (we are doing some name mangling in case of duplicated arguments, correct @nhaehnle ?)
• Add a test that shows how this plays with variadic arguments?

[nhaehnle]

As @tsymalla-AMD pointed out, you do need to fix the CI failures here.

[tsymalla]

As @tsymalla-AMD pointed out, you do need to fix the CI failures here.

You just need to take the ExampleDialect.h.inc and ExampleDialect.cpp.inc, which should be generated as part of the check-llvm-dialects target, and put it in the generated/ folder for the CI to pass.

[tsymalla]

Please add some tests (as mentioned above) that shows the name of the instruction is correctly set, and that this plays correctly with various cases. Thanks.

[JanRehders-AMD]

@tsymalla There was indeed a naming conflict causing this to not compile. I've added a fix for that (separate commit so it's easier to review but will be squashed) plus a test.

Here is the code produced by the conflict test I've added:

class InstNameConflictTestOp : public ::llvm::CallInst {
  static const ::llvm::StringLiteral s_name; //{"test.try.conflict"};

public:
  static bool classof(const ::llvm::CallInst *i) {
    return ::llvm_dialects::detail::isSimpleOperation(i, s_name);
  }
  static bool classof(const ::llvm::Value *v) {
    return ::llvm::isa<::llvm::CallInst>(v) &&
           classof(::llvm::cast<::llvm::CallInst>(v));
  }
  static InstNameConflictTestOp *create(::llvm_dialects::Builder &b,
                                        ::llvm::Value *instName,
                                        ::llvm::Value *inst_Name,
                                        const llvm::Twine &inst__name = "");

  bool verifier(::llvm::raw_ostream &errs);

  ::llvm::Value *getInstName();
  void setInstName(::llvm::Value *instName);
  ::llvm::Value *getInst_Name();
  void setInst_Name(::llvm::Value *inst__name);
};

and from the test single-vararg.td:

class SingleVarArgOp : public ::llvm::CallInst {
  static const ::llvm::StringLiteral s_name; //{"test.return"};

public:
  static bool classof(const ::llvm::CallInst *i) { return ::llvm_dialects::detail::isSimpleOperation(i, s_name); }
  static bool classof(const ::llvm::Value *v) {
    return ::llvm::isa<::llvm::CallInst>(v) && classof(::llvm::cast<::llvm::CallInst>(v));
  }
  static SingleVarArgOp *create(::llvm_dialects::Builder &b, ::llvm::ArrayRef<::llvm::Value *> args,
                                const llvm::Twine &inst__name = "");

  bool verifier(::llvm::raw_ostream &errs);

  ::llvm::iterator_range<::llvm::User::value_op_iterator> getArgs();
};

[nhaehnle]

Mostly LGTM, but please do extend test-builder.test (and ExampleMain.cpp) with a test for this.

[tsymalla-AMD]

LGTM (you need to resolve the merge conflict)

[JanRehders-AMD]

Please add some tests (as mentioned above) that shows the name of the instruction is correctly set, and that this plays correctly with various cases. Thanks.

From MainExample.cpp

  b.create<xd::HandleGetOp>("name.of.llvm.value");
  b.create<xd::InstNameConflictOp>(b.getInt32(1));
  b.create<xd::InstNameConflictOp>(b.getInt32(1), "name.foo");
  b.create<xd::InstNameConflictDoubleOp>(b.getInt32(1), b.getInt32(2));
  b.create<xd::InstNameConflictDoubleOp>(b.getInt32(1), b.getInt32(2), "bar");
  SmallVector<Value *> moreVarArgs = varArgs;
  b.create<xd::InstNameConflictVarargsOp>(moreVarArgs);
  b.create<xd::InstNameConflictVarargsOp>(moreVarArgs, "two.varargs");
  moreVarArgs.push_back(b.getInt32(3));
  b.create<xd::InstNameConflictVarargsOp>(moreVarArgs, "three.varargs");
  moreVarArgs.push_back(b.getInt32(4));
  b.create<xd::InstNameConflictVarargsOp>(moreVarArgs, "four.varargs");

produces

  %name.of.llvm.value = call target("xd.handle") @xd.handle.get()
  %19 = call i32 (...) @xd.inst.name.conflict(i32 1)
  %name.foo = call i32 (...) @xd.inst.name.conflict(i32 1)
  %20 = call i32 (...) @xd.inst.name.conflict.double(i32 1, i32 2)
  %bar = call i32 (...) @xd.inst.name.conflict.double(i32 1, i32 2)
  %21 = call i32 (...) @xd.inst.name.conflict.varargs(ptr %p1, i8 %p2)
  %two.varargs = call i32 (...) @xd.inst.name.conflict.varargs(ptr %p1, i8 %p2)
  %three.varargs = call i32 (...) @xd.inst.name.conflict.varargs(ptr %p1, i8 %p2, i32 3)
  %four.varargs = call i32 (...) @xd.inst.name.conflict.varargs(ptr %p1, i8 %p2, i32 3, i32 4)
  ret void