Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIRRTL] Integer Property folders assert in getAPSInt #8266

Open
mikeurbach opened this issue Feb 23, 2025 · 0 comments
Open

[FIRRTL] Integer Property folders assert in getAPSInt #8266

mikeurbach opened this issue Feb 23, 2025 · 0 comments
Assignees
Labels
bug Something isn't working FIRRTL Involving the `firrtl` dialect

Comments

@mikeurbach
Copy link
Contributor

@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()
@mikeurbach mikeurbach added bug Something isn't working FIRRTL Involving the `firrtl` dialect labels Feb 23, 2025
@mikeurbach mikeurbach self-assigned this Feb 23, 2025
mikeurbach referenced this issue Feb 23, 2025
This reverts commit f2e38e8.

In downstream testing, this folder appeared to cause assertions in
upstream MLIR to fail with the following:

```
llvm::APSInt mlir::IntegerAttr::getAPSInt() const: Assertion
`!getType().isSignlessInteger() && "Signless integers don't carry a
sign for APSInt"' failed.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working FIRRTL Involving the `firrtl` dialect
Projects
None yet
Development

No branches or pull requests

1 participant