[MLIR] Add the convergent attribute to LLVM Dialect #97709

FMarno · 2024-07-04T10:25:41Z

In order to use the convergent attribute in the GPUToLLVMSPV pass, I've added the attribute to the LLVM dialect.
Some details on the convergent attribute https://llvm.org/docs/ConvergentOperations.html#convergent-operations

github-actions · 2024-07-04T10:25:56Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-07-04T10:26:24Z

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-llvm

Author: Finlay (FMarno)

Changes

In order to use the convergent attribute in the GPUToLLVMSPV pass, I've added the attribute to the LLVM dialect.

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

5 Files Affected:

(modified) mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td (+1)
(modified) mlir/lib/Target/LLVMIR/ModuleImport.cpp (+3)
(modified) mlir/lib/Target/LLVMIR/ModuleTranslation.cpp (+2)
(modified) mlir/test/Dialect/LLVMIR/func.mlir (+6)
(modified) mlir/test/Target/LLVMIR/Import/function-attributes.ll (+6)

diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index fb3b8e77c492e..3774bda05eb2b 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -1425,6 +1425,7 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [
     UnitAttr:$dso_local,
     DefaultValuedAttr<CConv, "CConv::C">:$CConv,
     OptionalAttr<SymbolRefAttr>:$comdat,
+    OptionalAttr<UnitAttr>:$convergent,
     OptionalAttr<FlatSymbolRefAttr>:$personality,
     OptionalAttr<StrAttr>:$garbageCollector,
     OptionalAttr<ArrayAttr>:$passthrough,
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index cfcf33436a899..9b917db5e7dfe 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -1675,6 +1675,7 @@ static constexpr std::array kExplicitAttributes{
     StringLiteral("aarch64_pstate_sm_enabled"),
     StringLiteral("alwaysinline"),
     StringLiteral("approx-func-fp-math"),
+    StringLiteral("convergent"),
     StringLiteral("frame-pointer"),
     StringLiteral("no-infs-fp-math"),
     StringLiteral("no-nans-fp-math"),
@@ -1754,6 +1755,8 @@ void ModuleImport::processFunctionAttributes(llvm::Function *func,
     funcOp.setAlwaysInline(true);
   if (func->hasFnAttribute(llvm::Attribute::OptimizeNone))
     funcOp.setOptimizeNone(true);
+  if (func->hasFnAttribute(llvm::Attribute::Convergent))
+    funcOp.setConvergent(true);
 
   if (func->hasFnAttribute("aarch64_pstate_sm_enabled"))
     funcOp.setArmStreaming(true);
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index e0b1816e2dcfb..40196a5c760f9 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -1422,6 +1422,8 @@ static void convertFunctionAttributes(LLVMFuncOp func,
     llvmFunc->addFnAttr(llvm::Attribute::AlwaysInline);
   if (func.getOptimizeNoneAttr())
     llvmFunc->addFnAttr(llvm::Attribute::OptimizeNone);
+  if (func.getConvergentAttr())
+    llvmFunc->addFnAttr(llvm::Attribute::Convergent);
   convertFunctionMemoryAttributes(func, llvmFunc);
 }
 
diff --git a/mlir/test/Dialect/LLVMIR/func.mlir b/mlir/test/Dialect/LLVMIR/func.mlir
index 006f2f64a2727..d417942861940 100644
--- a/mlir/test/Dialect/LLVMIR/func.mlir
+++ b/mlir/test/Dialect/LLVMIR/func.mlir
@@ -287,6 +287,12 @@ module {
     // CHECK-SAME: attributes {no_signed_zeros_fp_math = true}
     llvm.return
   }
+
+  llvm.func @convergent_function() attributes {convergent} {
+    // CHECK: @convergent_function
+    // CHECK-SAME: attributes {convergent}
+    llvm.return
+  }
 }
 
 // -----
diff --git a/mlir/test/Target/LLVMIR/Import/function-attributes.ll b/mlir/test/Target/LLVMIR/Import/function-attributes.ll
index d9851e99fe33b..322ce6eadab4e 100644
--- a/mlir/test/Target/LLVMIR/Import/function-attributes.ll
+++ b/mlir/test/Target/LLVMIR/Import/function-attributes.ll
@@ -361,3 +361,9 @@ declare void @alwaysinline_attribute() alwaysinline
 ; CHECK-LABEL: @optnone_attribute
 ; CHECK-SAME: attributes {no_inline, optimize_none}
 declare void @optnone_attribute() noinline optnone
+
+// -----
+
+; CHECK-LABEL: @convergent_attribute
+; CHECK-SAME: attributes {convergent}
+declare void @convergent_attribute() convergent

victor-eds

Can we have a test in mlir/test/Dialect/LLVMIR/func.mlir? Also, modifying the operation assembly format in mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp (FuncOp::parse/print) would be better than the current syntax IMO.

gysit

LGTM

FMarno · 2024-07-04T13:46:39Z

Also, modifying the operation assembly format in mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp (FuncOp::parse/print) would be better than the current syntax IMO.

There current syntax is in line with other attributes like no_inline, always_inline, or alignment, which seems appropriate to me. examples in this test:

llvm-project/mlir/test/Target/LLVMIR/Import/function-attributes.ll

Lines 349 to 357 in a4ea8a4

    
           ; CHECK-LABEL: @noinline_attribute 
        
           ; CHECK-SAME: attributes {no_inline} 
        
           declare void @noinline_attribute() noinline 
        
           // ----- 
        
           ; CHECK-LABEL: @alwaysinline_attribute 
        
           ; CHECK-SAME: attributes {always_inline} 
        
           declare void @alwaysinline_attribute() alwaysinline

victor-eds · 2024-07-04T13:48:45Z

Also, modifying the operation assembly format in mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp (FuncOp::parse/print) would be better than the current syntax IMO.

There current syntax is in line with other attributes like no_inline, always_inline, or alignment, which seems appropriate to me. examples in this test:

llvm-project/mlir/test/Target/LLVMIR/Import/function-attributes.ll

Lines 349 to 357 in a4ea8a4

; CHECK-LABEL: @noinline_attribute

; CHECK-SAME: attributes {no_inline}

declare void @noinline_attribute() noinline

// -----

; CHECK-LABEL: @alwaysinline_attribute

; CHECK-SAME: attributes {always_inline}

declare void @alwaysinline_attribute() alwaysinline

Seems fair. Thanks! We'd only be missing an export to LLVM IR test then

added MLIR to LLVMIR test

FMarno · 2024-07-04T14:01:00Z

Seems fair. Thanks! We'd only be missing an export to LLVM IR test then

Thank you for the suggestion, that's been added now

github-actions · 2024-07-05T07:34:11Z

@FMarno Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

In order to use the convergent attribute in the GPUToLLVMSPV pass, I've added the attribute to the LLVM dialect. Some details on the convergent attribute https://llvm.org/docs/ConvergentOperations.html#convergent-operations

Add the convergent attribute to LLVM Dialect

a4ea8a4

llvmbot added mlir:llvm mlir labels Jul 4, 2024

victor-eds assigned FMarno Jul 4, 2024

victor-eds requested review from gysit and zero9178 July 4, 2024 10:34

victor-eds reviewed Jul 4, 2024

View reviewed changes

gysit approved these changes Jul 4, 2024

View reviewed changes

fixup! Add the convergent attribute to LLVM Dialect

99d87e7

added MLIR to LLVMIR test

victor-eds approved these changes Jul 4, 2024

View reviewed changes

victor-eds merged commit d6df018 into llvm:main Jul 5, 2024
7 checks passed

FMarno deleted the convergent_attr_llvm_dialect branch August 23, 2024 10:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MLIR] Add the convergent attribute to LLVM Dialect #97709

[MLIR] Add the convergent attribute to LLVM Dialect #97709

FMarno commented Jul 4, 2024 •

edited

Loading

github-actions bot commented Jul 4, 2024

llvmbot commented Jul 4, 2024 •

edited

Loading

victor-eds left a comment

gysit left a comment

FMarno commented Jul 4, 2024 •

edited

Loading

victor-eds commented Jul 4, 2024

FMarno commented Jul 4, 2024

github-actions bot commented Jul 5, 2024

[MLIR] Add the convergent attribute to LLVM Dialect #97709

[MLIR] Add the convergent attribute to LLVM Dialect #97709

Conversation

FMarno commented Jul 4, 2024 • edited Loading

github-actions bot commented Jul 4, 2024

llvmbot commented Jul 4, 2024 • edited Loading

victor-eds left a comment

Choose a reason for hiding this comment

gysit left a comment

Choose a reason for hiding this comment

FMarno commented Jul 4, 2024 • edited Loading

victor-eds commented Jul 4, 2024

FMarno commented Jul 4, 2024

github-actions bot commented Jul 5, 2024

FMarno commented Jul 4, 2024 •

edited

Loading

llvmbot commented Jul 4, 2024 •

edited

Loading

FMarno commented Jul 4, 2024 •

edited

Loading