Update llvm-project to 60a7d33106d3cd645d3100a8a935a1e3837f885d

[flemairen6]

llvm-project : 60a7d33106d3cd645d3100a8a935a1e3837f885d
stablehlo: 92203a9612dbdd6681ccd3e65bc61586c4290df1

[flemairen6]

For the errors, I'm seeing a bunch of the following, I'm starting to dig the problem but if someone already know where it comes from, I'll be glad to hear it!
``within split at /workdir/onnx-mlir/test/mlir/accelerators/nnpa/conversion/lower-all-to-llvm.mlir:407 offset :10:3: error: 'llvm.call' op missing var_callee_type attribute for vararg call

"zlow.conv2d"(%input, %kernel, %bias, %shape, %output) {kernel_shape = [5, 5], strides = [2, 2], padding_type = "SAME_PADDING", act_func = "ACT_RELU" } : (memref<2048xf16>, memref<2048xf16>, memref<2048xf16>, memref<7xi64>, memref<2048xf16>)-> ()

within split at /workdir/onnx-mlir/test/mlir/accelerators/nnpa/conversion/lower-all-to-llvm.mlir:407 offset :10:3: note: see current operation: "llvm.call"(%123, %124, %125, %103, %107, %109, %105) <{CConv = #llvm.cconv, TailCallKind = #llvm.tailcallkind, callee = @zdnn_init_pre_transformed_desc, fastmathFlags = #llvm.fastmath}> : (i64, i64, !llvm.ptr, i64, i64, i64, i64) -> ()``

[flemairen6]

So I identified the problematic change to be https://github.com/llvm/llvm-project/pull/99293/files however I have a hard time finding where it triggers in krnl-to-llvm.
For onnx-mlir/test/mlir/conversion/krnl_to_llvm/input_verification.mlir it seems to be a side effect from verify-input-tensors=true but it seems to trigger in some other places in nnpa tests on zlow.* operators.

I tried looking in DialectBuilder.cpp where llvm::CallOp are constructed but I could not find anything fishy. there. Trying my luck, @tungld do you have any idea where this could come from? I've seen your name pop up in different places related to those passes. Are there any places where we could be constructing llvm::CallOp without explicitly building them?
For the first test, I was also thinking that it could maybe come from the conversion of krnlEntryPointOp but I also didn't find anything fishy.

[AlexandreEichenberger]

@flemairen6 I will give it a try tomorrow to ferret out the needed changes.

[flemairen6]

@flemairen6 I will give it a try tomorrow to ferret out the needed changes.

@AlexandreEichenberger Have you had time to take a look at it already?

[AlexandreEichenberger]

Have you had time to take a look at it already?

on it now.

[AlexandreEichenberger]

Tried the simplest of the lit tests that fails

module {
  func.func @main_graph(%arg0: memref<3x4x5xf32>, %arg1: memref<?x4x5xf32>) -> memref<3x4x5xf32> {
    return %arg0 : memref<3x4x5xf32>
  }
  "krnl.entry_point"() {func = @main_graph, numInputs = 2 : i32, numOutputs = 1 : i32, signature = "[    { \22type\22 : \22f32\22 , \22dims\22 : [3 , 4 , 5] , \22name\22 : \22input0\22 }\0A ,    { \22type\22 : \22f32\22 , \22dims\22 : [-1 , 4 , 5] , \22name\22 : \22input1\22 }\0A\0A]\00@[   { \22type\22 : \22f32\22 , \22dims\22 : [3 , 4 , 5], \22name\22 : \22output0\22 }\0A\0A]\00"} : () -> ()
}

from onnx-mlir/test/mlir/conversion/krnl_to_llvm/input_verification.mlir compiled with onnx-mlir-opt --convert-krnl-to-llvm="verify-input-tensors=true" --canonicalize.

The code has essentially nothing. LLVM adds a bunch of external calls, and that is what it is choking on

flt_orig_model.mlir:8:3: note: see current operation: "llvm.call"(%4, %1) <{CConv = #llvm.cconv<ccc>, TailCallKind = #llvm.tailcallkind<none>, callee = @printf, fastmathFlags = #llvm.fastmath<none>}> : (!llvm.ptr, i64) -> ()

and the error printout lists the printf

  "llvm.func"() <{CConv = #llvm.cconv<ccc>, function_type = !llvm.func<void (ptr, ...)>, linkage = #llvm.linkage<external>, sym_name = "printf", visibility_ = 0 : i64}> ({
  }) : () -> ()

which is the very first function that is listed (and thus presumably added by our pass).

[AlexandreEichenberger]

Ok, I got it narrowed down a bit more. In onnnx-mlir, we can generate printf from the KrnlPrintOp. When creating the op, we know the format, as we print optionally one Value and at this time we limit the types to a very few, basically summarized below.

static StringRef getFormat(const Type &inputType) {
  StringRef format;
  TypeSwitch<Type>(inputType)
      .Case<Float16Type>([&](Float16Type) { format = "%g"; })
      .Case<Float32Type>([&](Float32Type) { format = "%f"; })
      .Case<Float64Type>([&](Float64Type) { format = "%f"; })
      .Case<IntegerType>([&](IntegerType type) {
        switch (type.getWidth()) {
        case 1:
        case 8:
        case 16:
        case 32:
          format = type.isUnsigned() ? "%u" : "%d";
          break;
        case 64:
          format = type.isUnsigned() ? "%llu" : "%lld";
          break;
        }
      })
      .Case<IndexType>([&](IndexType) { format = "%lld"; })
      .Case<onnx_mlir::krnl::StringType>(
          [&](onnx_mlir::krnl::StringType) { format = "%s"; })
      .Case<LLVM::LLVMPointerType>(
          [&](LLVM::LLVMPointerType) { format = "%s"; })
      .Default([&](Type type) {
        llvm::errs() << "type: " << type << "\n";
        llvm_unreachable("Unhandled type");
      });

  return format;
}

Now we need to provide the function type to LLVM in a setVarCaleeType. So we would know how to "remember" the type (or we can scan the string too), I just don't know how to create this type.

[AlexandreEichenberger]

Doing some searching in the code base, I found a way. Can you try this patch @flemairen6 ?

diff --git a/src/Conversion/KrnlToLLVM/KrnlPrint.cpp b/src/Conversion/KrnlToLLVM/KrnlPrint.cpp
index 9639f2ba..a1b631cf 100644
--- a/src/Conversion/KrnlToLLVM/KrnlPrint.cpp
+++ b/src/Conversion/KrnlToLLVM/KrnlPrint.cpp
@@ -55,9 +55,9 @@ public:
     Value formatSpecPtr = getPtrToGlobalString(formatSpec, loc, rewriter);
 
     if (input)
-      create.llvm.call({}, printfFuncRef, {formatSpecPtr, input});
+      create.llvm.call({}, printfFuncRef, {formatSpecPtr, input}, /*isVarArg*/ true);
     else
-      create.llvm.call({}, printfFuncRef, {formatSpecPtr});
+      create.llvm.call({}, printfFuncRef, {formatSpecPtr}, /*isVarArg*/ true);
 
     rewriter.eraseOp(op);
     return success();
diff --git a/src/Dialect/Mlir/DialectBuilder.cpp b/src/Dialect/Mlir/DialectBuilder.cpp
index cdeaa59a..d5bc6f05 100644
--- a/src/Dialect/Mlir/DialectBuilder.cpp
+++ b/src/Dialect/Mlir/DialectBuilder.cpp
@@ -1930,12 +1930,33 @@ void LLVMBuilder::br(ArrayRef<Value> destOperands, Block *destBlock) const {
   b().create<LLVM::BrOp>(loc(), destOperands, destBlock);
 }
 
+void LLVMBuilder::handleVarArgCall(LLVM::CallOp &callOp,
+    ArrayRef<Type> resultTypes, ArrayRef<Value> inputs) const {
+  // Define result type (void or 1).
+  Type resultType;
+  if (resultTypes.size() == 0 || isa<LLVM::LLVMVoidType>(resultTypes[0])) {
+    MLIRContext *ctx = b().getContext();
+    resultType = LLVM::LLVMVoidType::get(ctx);
+  } else {
+    resultType = resultTypes[0];
+  }
+  // Define input types.
+  llvm::SmallVector<Type, 4> inputTypes;
+  for (int64_t i = 0; i < (int64_t)inputs.size(); ++i)
+    inputTypes.emplace_back(inputs[i].getType());
+  auto typeSignature =
+      LLVM::LLVMFunctionType::get(resultType, inputTypes, /*is var arg*/ true);
+  callOp.setVarCalleeType(typeSignature);
+}
+
 Value LLVMBuilder::call(ArrayRef<Type> resultTypes, StringRef funcName,
-    ArrayRef<Value> inputs) const {
+    ArrayRef<Value> inputs, bool isVarArg) const {
   assert((resultTypes.size() == 0 || resultTypes.size() == 1) &&
          "LLVM:CallOp must return either 0 or 1 value");
   LLVM::CallOp callOp =
       b().create<LLVM::CallOp>(loc(), resultTypes, funcName, inputs);
+  if (isVarArg)
+    handleVarArgCall(callOp, resultTypes, inputs);
   // CallOp may return either 0 or 1 value.
   if (resultTypes.empty())
     return nullptr;
@@ -1943,11 +1964,13 @@ Value LLVMBuilder::call(ArrayRef<Type> resultTypes, StringRef funcName,
 }
 
 Value LLVMBuilder::call(ArrayRef<Type> resultTypes,
-    FlatSymbolRefAttr funcSymbol, ArrayRef<Value> inputs) const {
+    FlatSymbolRefAttr funcSymbol, ArrayRef<Value> inputs, bool isVarArg) const {
   assert((resultTypes.size() == 0 || resultTypes.size() == 1) &&
          "LLVM:CallOp must return either 0 or 1 value");
   LLVM::CallOp callOp =
       b().create<LLVM::CallOp>(loc(), resultTypes, funcSymbol, inputs);
+  if (isVarArg)
+    handleVarArgCall(callOp, resultTypes, inputs);
   // CallOp may return either 0 or 1 value.
   if (resultTypes.empty())
     return nullptr;
diff --git a/src/Dialect/Mlir/DialectBuilder.hpp b/src/Dialect/Mlir/DialectBuilder.hpp
index ed512dec..8a6ee058 100644
--- a/src/Dialect/Mlir/DialectBuilder.hpp
+++ b/src/Dialect/Mlir/DialectBuilder.hpp
@@ -627,10 +627,11 @@ struct LLVMBuilder final : DialectBuilder {
 
   // CallOp
   mlir::Value call(mlir::ArrayRef<mlir::Type> resultTypes,
-      llvm::StringRef funcName, mlir::ArrayRef<mlir::Value> inputs) const;
+      llvm::StringRef funcName, mlir::ArrayRef<mlir::Value> inputs,
+      bool isVarArg = false) const;
   mlir::Value call(mlir::ArrayRef<mlir::Type> resultTypes,
-      mlir::FlatSymbolRefAttr funcSymbol,
-      mlir::ArrayRef<mlir::Value> inputs) const;
+      mlir::FlatSymbolRefAttr funcSymbol, mlir::ArrayRef<mlir::Value> inputs,
+      bool isVarArg = false) const;
 
   // CondBrOp
   void condBr(mlir::Value cond, mlir::Block *trueBlock,
@@ -649,7 +650,8 @@ struct LLVMBuilder final : DialectBuilder {
   mlir::Value extractValue(mlir::Type resultType, mlir::Value container,
       llvm::ArrayRef<int64_t> position) const;
 
-  // FuncOp
+  // FuncOp (assume non-variadic functions, otherwise add support like in
+  // seen in `call` in this file).
   mlir::LLVM::LLVMFuncOp func(llvm::StringRef name, mlir::Type type,
       bool createUniqueFunc = false) const;
 
@@ -752,6 +754,12 @@ struct LLVMBuilder final : DialectBuilder {
     }
     return symbol + postfix;
   }
+
+private:
+  // Support for calling vararg functions; add the necessary type.
+  void handleVarArgCall(mlir::LLVM::CallOp &callOp,
+      mlir::ArrayRef<mlir::Type> resultTypes,
+      mlir::ArrayRef<mlir::Value> inputs) const;
 };
 
 //===----------------------------------------------------------------------===//

[flemairen6]

Thanks a lot Alexandre, it seems to be working for the simple test. I'll apply the same kind of changes for the NNPA tests failing and it should be enough to have this PR green

[AlexandreEichenberger]

LGTM, thanks for catching the extra cases and fixing the lit tests. Much much appreciated. I also learned more about LLVM lowering, always good to learn a bit more.

[jenkins-droid]

Jenkins Linux s390x Build #15321 [push] Update llvm-project to 6... started at 09:56

[jenkins-droid]

Jenkins Linux amd64 Build #15316 [push] Update llvm-project to 6... started at 08:56

[jenkins-droid]

Jenkins Linux ppc64le Build #14346 [push] Update llvm-project to 6... started at 09:56

[jenkins-droid]

Jenkins Linux amd64 Build #15316 [push] Update llvm-project to 6... passed after 2 hr 21 min

[jenkins-droid]

Jenkins Linux s390x Build #15321 [push] Update llvm-project to 6... passed after 2 hr 59 min

[jenkins-droid]

Jenkins Linux ppc64le Build #14346 [push] Update llvm-project to 6... passed after 3 hr 31 min