[mlir] Do not print back 0 alignment in LLVM dialect 'alloca' op

[ftynse]

Summary

The alignment attribute in the 'alloca' op treats the '0' value as 'unset'.
When parsing the custom form of the 'alloca' op, ignore the alignment attribute
with if its value is '0' instead of actually creating it and producing a
slightly different textually yet equivalent semantically form in the output.

[rriddle]

Nice, was just looking for something like this.

mlir/include/mlir/IR/OperationSupport.h

261

nit: was erased

[ftynse]

Address review

mlir/include/mlir/IR/OperationSupport.h

public:
   Optional<NamedAttribute> getNamed(StringRef name) const;
   Optional<NamedAttribute> getNamed(Identifier name) const;
 
   /// If the an attribute exists with the specified name, change it to the new
   /// value.  Otherwise, add a new attribute with the specified name/value.
   void set(Identifier name, Attribute value);
   void set(StringRef name, Attribute value);
 
+  /// Erase the attribute with the given name from the list. Return the
+  /// attribute that was erased, or nullptr if there was no attribute with such
+  /// name.
+  Attribute erase(Identifier name);
+  Attribute erase(StringRef name);
+
   const_iterator begin() const { return attrs.begin(); }
   const_iterator end() const { return attrs.end(); }
 
   NamedAttrList &operator=(const SmallVectorImpl<NamedAttribute> &rhs);
   operator ArrayRef<NamedAttribute>() const;
   operator MutableDictionaryAttr() const;
 
 private:
   /// Return whether the attributes are sorted.
   bool isSorted() const { return dictionarySorted.getInt(); }
 
+  /// Erase the attribute at the given iterator position.
+  Attribute eraseImpl(SmallVectorImpl<NamedAttribute>::iterator it);
+
   // These are marked mutable as they may be modified (e.g., sorted)
   mutable SmallVector<NamedAttribute, 4> attrs;
   // Pair with cached DictionaryAttr and status of whether attrs is sorted.
   // Note: just because sorted does not mean a DictionaryAttr has been created
   // but the case where there is a DictionaryAttr but attrs isn't sorted should
   // not occur.
   mutable llvm::PointerIntPair<Attribute, 1, bool> dictionarySorted;
 };

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

static ParseResult parseAllocaOp(OpAsmParser &parser, OperationState &result) {
   Type type, elemType;
   llvm::SMLoc trailingTypeLoc;
   if (parser.parseOperand(arraySize) || parser.parseKeyword("x") ||
       parser.parseType(elemType) ||
       parser.parseOptionalAttrDict(result.attributes) || parser.parseColon() ||
       parser.getCurrentLocation(&trailingTypeLoc) || parser.parseType(type))
     return failure();
 
+  Optional<NamedAttribute> alignmentAttr =
+      result.attributes.getNamed("alignment");
+  if (alignmentAttr.hasValue()) {
+    auto alignmentInt = alignmentAttr.getValue().second.dyn_cast<IntegerAttr>();
+    if (!alignmentInt)
+      return parser.emitError(parser.getNameLoc(),
+                              "expected integer alignment");
+    if (alignmentInt.getValue().isNullValue())
+      result.attributes.erase("alignment");
+  }
+
   // Extract the result type from the trailing function type.
   auto funcType = type.dyn_cast<FunctionType>();
   if (!funcType || funcType.getNumInputs() != 1 ||
       funcType.getNumResults() != 1)
     return parser.emitError(
         trailingTypeLoc,
         "expected trailing function type with one argument and one result");
 

mlir/lib/IR/OperationSupport.cpp

void NamedAttrList::set(Identifier name, Attribute value) {
   dictionarySorted.setPointer(nullptr);
   attrs.insert(it, {name, value});
 }
 void NamedAttrList::set(StringRef name, Attribute value) {
   assert(value && "setting null attribute not supported");
   return set(mlir::Identifier::get(name, value.getContext()), value);
 }
 
+Attribute
+NamedAttrList::eraseImpl(SmallVectorImpl<NamedAttribute>::iterator it) {
+  if (it == attrs.end())
+    return nullptr;
+
+  // Erasing does not affect the sorted property.
+  Attribute attr = it->second;
+  attrs.erase(it);
+  dictionarySorted.setPointer(nullptr);
+  return attr;
+}
+
+Attribute NamedAttrList::erase(Identifier name) {
+  return eraseImpl(findAttr(attrs, name, isSorted()));
+}
+
+Attribute NamedAttrList::erase(StringRef name) {
+  return eraseImpl(findAttr(attrs, name, isSorted()));
+}
+
 NamedAttrList &
 NamedAttrList::operator=(const SmallVectorImpl<NamedAttribute> &rhs) {
   assign(rhs.begin(), rhs.end());
   return *this;
 }
 
 NamedAttrList::operator ArrayRef<NamedAttribute>() const { return attrs; }
 

mlir/test/Dialect/LLVMIR/invalid.mlir

 
 func @alloca_non_function_type() {
   // expected-error@+1 {{expected trailing function type with one argument and one result}}
   llvm.alloca %size x !llvm.i32 : !llvm.ptr<i32>
 }
 
 // -----
 
+func @alloca_non_integer_alignment() {
+  // expected-error@+1 {{expected integer alignment}}
+  llvm.alloca %size x !llvm.i32 {alignment = 3.0} : !llvm.ptr<i32>
+}
+
+// -----
+
 func @gep_missing_input_result_type(%pos : !llvm.i64, %base : !llvm.ptr<float>) {
   // expected-error@+1 {{2 operands present, but expected 0}}
   llvm.getelementptr %base[%pos] : () -> ()
 }
 
 // -----
 
 func @gep_missing_input_type(%pos : !llvm.i64, %base : !llvm.ptr<float>) {