[Mlir-commits] [mlir] [mlir][llvm] Fix loop annotation parser (PR #78266)

[Tobias Gysi]

https://github.com/gysit created https://github.com/llvm/llvm-project/pull/78266

This revision moves the array ref field of the LoopAnnotation attribute to the end of the struct to enable printing and parsing of the attribute. Previously, the parsing could fail in the presence of a start or end loc.

>From 197b6c19a4db7ce46997d28e2dc0cdd7fb69ec26 Mon Sep 17 00:00:00 2001
From: Tobias Gysi <tobias.gysi at nextsilicon.com>
Date: Tue, 16 Jan 2024 12:32:48 +0000
Subject: [PATCH] [mlir][llvm] Fix loop annotation parser

This revision moves the array ref field of the LoopAnnotation
attribute to the end of the struct to enable printing and
parsing of the attribute. Previously, the parsing could fail
in the presence of a start or end loc.
---
 .../mlir/Dialect/LLVMIR/LLVMAttrDefs.td       |  4 ++--
 .../Target/LLVMIR/LoopAnnotationImporter.cpp  |  4 ++--
 mlir/test/Dialect/LLVMIR/loop-metadata.mlir   | 19 +++++++++++++------
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index 86ba9f4d3840bd..a831b076fb864f 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -225,9 +225,9 @@ def LoopAnnotationAttr : LLVM_Attr<"LoopAnnotation", "loop_annotation"> {
     OptionalParameter<"LoopUnswitchAttr">:$unswitch,
     OptionalParameter<"BoolAttr">:$mustProgress,
     OptionalParameter<"BoolAttr">:$isVectorized,
-    OptionalArrayRefParameter<"AccessGroupAttr">:$parallelAccesses,
     OptionalParameter<"FusedLoc">:$startLoc,
-    OptionalParameter<"FusedLoc">:$endLoc
+    OptionalParameter<"FusedLoc">:$endLoc,
+    OptionalArrayRefParameter<"AccessGroupAttr">:$parallelAccesses
   );
 
   let assemblyFormat = "`<` struct(params) `>`";
diff --git a/mlir/lib/Target/LLVMIR/LoopAnnotationImporter.cpp b/mlir/lib/Target/LLVMIR/LoopAnnotationImporter.cpp
index 8dc53a4326ed44..e4905423347a21 100644
--- a/mlir/lib/Target/LLVMIR/LoopAnnotationImporter.cpp
+++ b/mlir/lib/Target/LLVMIR/LoopAnnotationImporter.cpp
@@ -461,8 +461,8 @@ LoopAnnotationAttr LoopMetadataConversion::convert() {
   return createIfNonNull<LoopAnnotationAttr>(
       ctx, disableNonForced, vecAttr, interleaveAttr, unrollAttr,
       unrollAndJamAttr, licmAttr, distributeAttr, pipelineAttr, peeledAttr,
-      unswitchAttr, mustProgress, isVectorized, parallelAccesses, startLoc,
-      endLoc);
+      unswitchAttr, mustProgress, isVectorized, startLoc, endLoc,
+      parallelAccesses);
 }
 
 LoopAnnotationAttr
diff --git a/mlir/test/Dialect/LLVMIR/loop-metadata.mlir b/mlir/test/Dialect/LLVMIR/loop-metadata.mlir
index 97d3aa8f7ec759..6b4c3217d5c72d 100644
--- a/mlir/test/Dialect/LLVMIR/loop-metadata.mlir
+++ b/mlir/test/Dialect/LLVMIR/loop-metadata.mlir
@@ -85,28 +85,35 @@ llvm.func @loop_annotation() {
 
 #di_file = #llvm.di_file<"metadata-loop.ll" in "/">
 
-// CHECK: #[[START_LOC:.*]] = loc("loop-metadata.mlir":42:4)
+// CHECK-DAG: #[[START_LOC:.*]] = loc("loop-metadata.mlir":42:4)
 #loc1 = loc("loop-metadata.mlir":42:4)
-// CHECK: #[[END_LOC:.*]] = loc("loop-metadata.mlir":52:4)
+// CHECK-DAG: #[[END_LOC:.*]] = loc("loop-metadata.mlir":52:4)
 #loc2 = loc("loop-metadata.mlir":52:4)
 
 #di_compile_unit = #llvm.di_compile_unit<id = distinct[0]<>, sourceLanguage = DW_LANG_C, file = #di_file, isOptimized = false, emissionKind = None>
-// CHECK: #[[SUBPROGRAM:.*]] = #llvm.di_subprogram<
+// CHECK-DAG: #[[SUBPROGRAM:.*]] = #llvm.di_subprogram<
 #di_subprogram = #llvm.di_subprogram<compileUnit = #di_compile_unit, scope = #di_file, name = "loop_locs", file = #di_file, subprogramFlags = Definition>
 
-// CHECK: #[[START_LOC_FUSED:.*]] = loc(fused<#[[SUBPROGRAM]]>[#[[START_LOC]]]
+// CHECK-DAG: #[[START_LOC_FUSED:.*]] = loc(fused<#[[SUBPROGRAM]]>[#[[START_LOC]]]
 #start_loc_fused = loc(fused<#di_subprogram>[#loc1])
-// CHECK: #[[END_LOC_FUSED:.*]] = loc(fused<#[[SUBPROGRAM]]>[#[[END_LOC]]]
+// CHECK-DAG: #[[END_LOC_FUSED:.*]] = loc(fused<#[[SUBPROGRAM]]>[#[[END_LOC]]]
 #end_loc_fused= loc(fused<#di_subprogram>[#loc2])
 
+// CHECK-DAG: #[[GROUP1:.*]] = #llvm.access_group<id = {{.*}}>
+// CHECK-DAG: #[[GROUP2:.*]] = #llvm.access_group<id = {{.*}}>
+#group1 = #llvm.access_group<id = distinct[0]<>>
+#group2 = #llvm.access_group<id = distinct[1]<>>
+
 // CHECK: #[[LOOP_ANNOT:.*]] = #llvm.loop_annotation<
 // CHECK-DAG: disableNonforced = false
 // CHECK-DAG: startLoc = #[[START_LOC_FUSED]]
 // CHECK-DAG: endLoc = #[[END_LOC_FUSED]]
+// CHECK-DAG: parallelAccesses = #[[GROUP1]], #[[GROUP2]]>
 #loopMD = #llvm.loop_annotation<disableNonforced = false,
         mustProgress = true,
         startLoc = #start_loc_fused,
-        endLoc = #end_loc_fused>
+        endLoc = #end_loc_fused,
+        parallelAccesses = #group1, #group2>
 
 // CHECK: llvm.func @loop_annotation_with_locs
 llvm.func @loop_annotation_with_locs() {