[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)

[Tobias Gysi via cfe-commits]

@@ -0,0 +1,9 @@
+; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s

gysit wrote:

> To me, it would make sense if mlir-translate didn't care - it doesn't have to 
> interpret these attributes, does it?

It doesn't do anything with them except for running the LLVM verifier. If 
things pass on Aarch64, then the guards are not necessary. 

https://github.com/llvm/llvm-project/pull/96688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)

[Tobias Gysi via cfe-commits]

@@ -0,0 +1,9 @@
+; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s

gysit wrote:

Interesting!

Did you build your LLVM with the x86 target enabled (besides Aarch64)? If not, 
`mlir-translate` may indeed be fine with the attribute independent of the 
target architecture. Note that `mlir-translate` runs `llvm::verifyModule` which 
I thought could be unhappy with unsupported target specific attributes.


https://github.com/llvm/llvm-project/pull/96688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)

[Tobias Gysi via cfe-commits]

@@ -0,0 +1,7 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s

gysit wrote:

This may require x86-registered-target as well?

https://github.com/llvm/llvm-project/pull/96688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [flang] [mlir] [flang] Retry add basic -mtune support (PR #96688)

[Tobias Gysi via cfe-commits]

@@ -0,0 +1,9 @@
+; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s

gysit wrote:

Should this test also be guarded with `REQUIRES: x86-registered-target`?

https://github.com/llvm/llvm-project/pull/96688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [flang] [mlir] [flang] Add basic -mtune support (PR #95043)

[Tobias Gysi via cfe-commits]

@@ -1796,6 +1797,10 @@ void 
ModuleImport::processFunctionAttributes(llvm::Function *func,
   attr.isStringAttribute())
 funcOp.setTargetCpuAttr(StringAttr::get(context, attr.getValueAsString()));
 
+  if (llvm::Attribute attr = func->getFnAttribute("tune-cpu");
+  attr.isStringAttribute())
+funcOp.setTuneCpuAttr(StringAttr::get(context, attr.getValueAsString()));

gysit wrote:

Nice!

Separate tests are fine.

https://github.com/llvm/llvm-project/pull/95043
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [flang] [mlir] [flang] Add basic -mtune support (PR #95043)

[Tobias Gysi via cfe-commits]

@@ -1796,6 +1797,10 @@ void 
ModuleImport::processFunctionAttributes(llvm::Function *func,
   attr.isStringAttribute())
 funcOp.setTargetCpuAttr(StringAttr::get(context, attr.getValueAsString()));
 
+  if (llvm::Attribute attr = func->getFnAttribute("tune-cpu");
+  attr.isStringAttribute())
+funcOp.setTuneCpuAttr(StringAttr::get(context, attr.getValueAsString()));

gysit wrote:

Could you also add a small test that verifies the import / export works 
independent from Flang?

The target_cpu attributes are tested here:
```
llvm-project/mlir/test/Target/LLVMIR/target-cpu.mlir
llvm-project/mlir/test/Target/LLVMIR/Import/target-cpu.ll
```
It may make sense to add the tests in the same files and maybe rename them to 
target-and-tune-cpu.mlir/ll or have a separate set of files if the attributes 
are unrelated (which is not my impression).

Thanks!

https://github.com/llvm/llvm-project/pull/95043
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits