[Mlir-commits] [mlir] [MLIR][IR] Fix InProgressAliasInfo init for non-alias (PR #109013)

[Billy Zhu]

https://github.com/zyx-billy created https://github.com/llvm/llvm-project/pull/109013

When visiting an attr/type that is NoAlias, the created `InProgressAliasInfo` was not getting its `canBeDeferred` and `isType` fields set. Not setting `canBeDeferred` when it should be true breaks the assumption that all nested elements are also false. This will cause problems when at a later point the attr/type needs to converted by `markAliasNonDeferrable`, as recursion will stop when a `canBeDeferred=false` attr/type is reached, and the nested elements are not flipped. This causes nested elements to be printed later in the textual IR and cannot be loaded back in.

>From 030f4872a6f6cff52378b7cb7f767f422dba79a3 Mon Sep 17 00:00:00 2001
From: Billy Zhu <billyzhu at modular.com>
Date: Mon, 16 Sep 2024 19:31:45 -0700
Subject: [PATCH 1/2] always set canBeDeferred

---
 mlir/lib/IR/AsmPrinter.cpp | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index c7ed158aabb6e7..32182c083a8a38 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/mlir/lib/IR/AsmPrinter.cpp
@@ -584,9 +584,8 @@ class AliasInitializer {
   struct InProgressAliasInfo {
     InProgressAliasInfo()
         : aliasDepth(0), isType(false), canBeDeferred(false) {}
-    InProgressAliasInfo(StringRef alias, bool isType, bool canBeDeferred)
-        : alias(alias), aliasDepth(1), isType(isType),
-          canBeDeferred(canBeDeferred) {}
+    InProgressAliasInfo(StringRef alias)
+        : alias(alias), aliasDepth(1), isType(false), canBeDeferred(false) {}
 
     bool operator<(const InProgressAliasInfo &rhs) const {
       // Order first by depth, then by attr/type kind, and then by name.
@@ -1096,6 +1095,8 @@ std::pair<size_t, size_t> AliasInitializer::visitImpl(
 
   // Try to generate an alias for this value.
   generateAlias(value, it->second, canBeDeferred);
+  it->second.isType = std::is_base_of_v<Type, T>;
+  it->second.canBeDeferred = canBeDeferred;
 
   // Print the value, capturing any nested elements that require aliases.
   SmallVector<size_t> childAliases;
@@ -1153,8 +1154,7 @@ void AliasInitializer::generateAlias(T symbol, InProgressAliasInfo &alias,
       sanitizeIdentifier(nameBuffer, tempBuffer, /*allowedPunctChars=*/"$_-",
                          /*allowTrailingDigit=*/false);
   name = name.copy(aliasAllocator);
-  alias = InProgressAliasInfo(name, /*isType=*/std::is_base_of_v<Type, T>,
-                              canBeDeferred);
+  alias = InProgressAliasInfo(name);
 }
 
 //===----------------------------------------------------------------------===//

>From 03e0b954728242572359923df82b71777f5f0a65 Mon Sep 17 00:00:00 2001
From: Billy Zhu <billyzhu at modular.com>
Date: Mon, 16 Sep 2024 19:58:41 -0700
Subject: [PATCH 2/2] add test

---
 mlir/test/IR/print-attr-type-aliases.mlir | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/mlir/test/IR/print-attr-type-aliases.mlir b/mlir/test/IR/print-attr-type-aliases.mlir
index 27c5a75addbb59..eb80ba1076e445 100644
--- a/mlir/test/IR/print-attr-type-aliases.mlir
+++ b/mlir/test/IR/print-attr-type-aliases.mlir
@@ -1,6 +1,6 @@
-// RUN: mlir-opt %s -split-input-file | FileCheck %s
+// RUN: mlir-opt %s -split-input-file -mlir-print-debuginfo | FileCheck %s
 // Verify printer of type & attr aliases.
-// RUN: mlir-opt %s -split-input-file | mlir-opt -split-input-file | FileCheck %s
+// RUN: mlir-opt %s -split-input-file -mlir-print-debuginfo | mlir-opt -split-input-file -mlir-print-debuginfo | FileCheck %s
 
 // CHECK-DAG: #test2Ealias = "alias_test:dot_in_name"
 "test.op"() {alias_test = "alias_test:dot_in_name"} : () -> ()
@@ -71,3 +71,16 @@
 "test.op"() {attr = #test.conditional_alias<#unalias_me>} : () -> ()
 // CHECK-NEXT: #test.conditional_alias<#test2Ealias>
 "test.op"() {attr = #test.conditional_alias<#keep_aliased>} : () -> ()
+
+// -----
+
+// Check that a deferred no_alias attr can be un-deferred.
+
+#keep_aliased = "alias_test:dot_in_name"
+#cond_alias = #test.conditional_alias<#keep_aliased>
+#no_alias = loc(fused<#cond_alias>["test.mlir":1:1])
+
+// CHECK: #[[TEST_ALIAS:.+]] = "alias_test:dot_in_name"
+// CHECK: fused<#test.conditional_alias<#[[TEST_ALIAS]]>
+// CHECK: "test.op"
+"test.op"() {attr = #no_alias} : () -> () loc(fused<#no_alias>["test.mlir":0:0])