[mlir][Target] Make nvptxcompiler passed options high priority to keep consistent with ptxas behavior

[MikaOvO]

When using ptxas to do ptx->cubin, the options passed (via cmd) have higher priority than the gpuModule target (such as opt-level). When using nvptxcompiler, it is the opposite and we need to be consistent.

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-llvm

Author: Zichen Lu (MikaOvO)

Changes

When using ptxas to do ptx->cubin, the options passed (via cmd) have higher priority than the gpuModule target (such as opt-level). When using nvptxcompiler, it is the opposite and we need to be consistent.

---

Full diff: https://github.com/llvm/llvm-project/pull/121036.diff

1 Files Affected:

• (modified) mlir/lib/Target/LLVM/NVVM/Target.cpp (+2-1)

diff --git a/mlir/lib/Target/LLVM/NVVM/Target.cpp b/mlir/lib/Target/LLVM/NVVM/Target.cpp
index bca26e3a0e84a9..0f630f56af301e 100644
--- a/mlir/lib/Target/LLVM/NVVM/Target.cpp
+++ b/mlir/lib/Target/LLVM/NVVM/Target.cpp
@@ -484,7 +484,8 @@ NVPTXSerializer::compileToBinaryNVPTX(const std::string &ptxCode) {
   std::string optLevel = std::to_string(this->optLevel);
   std::pair<llvm::BumpPtrAllocator, SmallVector<const char *>> cmdOpts =
       targetOptions.tokenizeCmdOptions();
-  cmdOpts.second.append(
+  cmdOpts.second.insert(
+      cmdOpts.second.begin(),
       {"-arch", getTarget().getChip().data(), "--opt-level", optLevel.c_str()});
 
   // Create the compiler handle.

[grypp]

PR looks good, but we need a test as it's llvm rule

[MikaOvO]

This code works when the macro MLIR_ENABLE_NVPTXCOMPILER=ON. Do we have the test env with this flag?

[grypp]

This code works when the macro MLIR_ENABLE_NVPTXCOMPILER=ON. Do we have the test env with this flag?

Maybe we can add one and test also this PR? What do you think?

I think it's okay just to test compilation, we don't really need to run the test.

[MikaOvO]

Could you please give me some guidance on how to add compilation test? Thanks!

[grypp]

If you pass -debug-only=serialize-to-binary, it prints debug messages. Maybe we can use this to test? what do you think?

I think you can compile code with mlir-opt %s --gpu-module-to-binary="format=binary"

[MikaOvO]

If I understand correctly, mlir-opt %s --gpu-module-to-binary="format=binary" is a runtime test. But I think we need to add a compilation test (build mlir with flag -DMLIR_ENABLE_NVPTXCOMPILER=ON), then we can test this change.

[grypp]

Sorry, this PR took too long. Let's try to land it as quick as possible.

But I think we need to add a compilation test (build mlir with flag -DMLIR_ENABLE_NVPTXCOMPILER=ON), then we can test this change.

Do we know that we build mlir with this option enabled? If not, we cannot really test this PR right?

Maybe @joker-eph knows

[MikaOvO]

I think we cannot really test this PR... But I have tested it locally. @joker-eph Could you help double-check & merge it?

[joker-eph]

It's hard to test the combination of all the build options in CI, there are too many... This one does not seem enabled on a bot: https://github.com/search?q=repo%3Allvm%2Fllvm-zorg%20MLIR_ENABLE_NVPTXCOMPILER&type=code

You could still add a test that would fail when this configuration is enabled though.

[MikaOvO]

Sorry, I still don't know how to add a test, could you explain it? Thanks!

[grypp]

What happens if you "mlir-opt %s --gpu-module-to-binary="format=binary -debug-only=serialize-to-binary"? I think it should print args that we pass it nvptxcompiler? You write a "//CHECK: ..." on the result of debug-only=serialize-to-binary.

We can add the test, but keep it disabled until we enable nvptxcompiler in the builtbit.