[FIRRTL] Integer Property folders assert in getAPSInt

[mikeurbach]

@seldridge did some digging:

The issue appears to be that we have an i8 attribute and it is being converted to an APSInt:

[firtool]   Running "firrtl-imconstprop"
Assertion failed: (!getType().isSignlessInteger() && "Signless integers don't carry a sign for APSInt"), function getAPSInt, file BuiltinAttributes.cpp, line 376.
Process 26737 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = hit program assert
    frame #4: 0x0000000100c0eeb8 firtool`mlir::IntegerAttr::getAPSInt() const (.cold.1) at BuiltinAttributes.cpp:375:3 [opt]
   372 	/// Return the value as an APSInt which carries the signed from the type of
   373 	/// the attribute.  This traps on signless integers types!
   374 	APSInt IntegerAttr::getAPSInt() const {
-> 375 	  assert(!getType().isSignlessInteger() &&
   376 	         "Signless integers don't carry a sign for APSInt");
   377 	  return APSInt(getValue(), getType().isUnsignedInteger());
   378 	}
Target 0: (firtool) stopped.
warning: firtool was compiled with optimization - stepping may behave oddly; variables may not be available.
(lldb) frame select 6
frame #6: 0x000000010062b1b4 firtool`circt::firrtl::IntegerAddOp::fold(circt::firrtl::IntegerAddOpGenericAdaptor<llvm::ArrayRef<mlir::Attribute>>) at FIRRTLFolds.cpp:176:17 [opt]
   173 	  if (auto attr = dyn_cast<BoolAttr>(operand))
   174 	    return APSInt(APInt(1, attr.getValue()));
   175 	  if (auto attr = dyn_cast<IntegerAttr>(operand))
-> 176 	    return attr.getAPSInt();
   177 	  return {};
   178 	}
   179 	
(lldb) print attr
(mlir::IntegerAttr) {
  mlir::Attribute::AttrBase<IntegerAttr, ::mlir::Attribute, detail::IntegerAttrStorage, mlir::TypedAttr::Trait> = {
    mlir::Attribute = {
      impl = 0x000000011c714548
    }
  }
}
(lldb) print attr->dump()
2 : i8
  Evaluated this expression after applying Fix-It(s):
    attr.dump()

[mtsokol]

@mikeurbach I took a look at it and it seems that for other dialects, Ops (and respective folders) provide additionally a signedness, like in HWArith.

In my folders to construct a return attribute type I use IntegerType::get(context, resultWidth) - the same way as the existing IntegerShlOp::fold that doesn't cause trouble.

It seems that for add and mul IntegerType::get(getContext(), resultWidth) results that output attribute is sometime signless, for the tests that you run.

My proposition is that I can make it IntegerType::get(getContext(), resultWidth, signedness) and replicate AddOp/MulOp::inferReturnTypes from other dialects for add and sub folders in FIRRTLFolds.cpp (I think it's one more thing that these folds should also do).

[mikeurbach]

I think that makes sense. I'm trying to get a reduced test case to show the issue and verify the fix.