From 64c6d3946c6154456e0986d1fb593119e96b80e9 Mon Sep 17 00:00:00 2001
From: Jacques Pienaar <jpienaar@google.com>
Date: Sat, 17 Nov 2018 08:24:07 -0800
Subject: [PATCH] Change pretty printing of constant so that the attributes
 precede the value.

This does create an inconsistency between the print formats (e.g., attributes are normally before operands) but fixes an invalid parsing & keeps constant uniform wrt itself (function or int attributes have type at same place). And specifying the specific type for a int/float attribute might get revised shortly.

Also add test to verify that output printed can be parsed again.

PiperOrigin-RevId: 221923893
---
 mlir/lib/IR/BuiltinOps.cpp | 9 ++++++---
 mlir/test/IR/core-ops.mlir | 8 +++++---
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/mlir/lib/IR/BuiltinOps.cpp b/mlir/lib/IR/BuiltinOps.cpp
index ff2bff460924..fe1c4024c5e7 100644
--- a/mlir/lib/IR/BuiltinOps.cpp
+++ b/mlir/lib/IR/BuiltinOps.cpp
@@ -314,9 +314,12 @@ void ConstantOp::build(Builder *builder, OperationState *result,
 }
 
 void ConstantOp::print(OpAsmPrinter *p) const {
-  *p << "constant " << getValue();
+  *p << "constant ";
   p->printOptionalAttrDict(getAttrs(), /*elidedAttrs=*/"value");
 
+  if (getAttrs().size() > 1)
+    *p << ' ';
+  *p << getValue();
   if (!getValue().isa<FunctionAttr>())
     *p << " : " << getType();
 }
@@ -325,8 +328,8 @@ bool ConstantOp::parse(OpAsmParser *parser, OperationState *result) {
   Attribute valueAttr;
   Type type;
 
-  if (parser->parseAttribute(valueAttr, "value", result->attributes) ||
-      parser->parseOptionalAttributeDict(result->attributes))
+  if (parser->parseOptionalAttributeDict(result->attributes) ||
+      parser->parseAttribute(valueAttr, "value", result->attributes))
     return true;
 
   // 'constant' taking a function reference doesn't get a redundant type
diff --git a/mlir/test/IR/core-ops.mlir b/mlir/test/IR/core-ops.mlir
index 289b7bf43818..3e876e6a57f0 100644
--- a/mlir/test/IR/core-ops.mlir
+++ b/mlir/test/IR/core-ops.mlir
@@ -1,4 +1,6 @@
 // RUN: mlir-opt %s | FileCheck %s
+// Verify the printed output can be parsed.
+// RUN: mlir-opt %s | mlir-opt | FileCheck %s
 
 // CHECK: #map0 = (d0) -> (d0 + 1)
 
@@ -71,13 +73,13 @@ bb42(%t: tensor<4x4x?xf32>, %f: f32, %i: i32, %idx : index):
   %i6 = muli %i2, %i2 : i32
 
   // CHECK: %c42_i32 = constant 42 : i32
-  %x = "constant"(){value: 42: i32} : () -> i32
+  %x = "constant"(){value: 42 : i32} : () -> i32
 
   // CHECK: %c42_i32_0 = constant 42 : i32
   %7 = constant 42 : i32
 
-  // CHECK: %c43 = constant 43 {crazy: "foo"} : index
-  %8 = constant 43: index {crazy: "foo"}
+  // CHECK: %c43 = constant {crazy: "foo"} 43 : index
+  %8 = constant {crazy: "foo"} 43: index
 
   // CHECK: %cst = constant 4.300000e+01 : bf16
   %9 = constant 43.0 : bf16