[IntegerAttr] Add helpers for working with LLVM's APSInt type.

[lattner]

Summary

The FIRRTL dialect in CIRCT uses inherently signful types, and APSInt
is the best way to model that. Add a couple of helpers that make it
easier to work with an IntegerAttr that carries a sign.

This follows the example of getZExt() and getSExt() which assert when
the underlying type of the attribute is unexpected. In this case
we assert fail when the underlying type of the attribute is signless.

This is strictly additive, so it is NFC. It is tested in the CIRCT
repo.

[rriddle]

mlir/lib/IR/BuiltinAttributes.cpp

265–266

As an aside, I'm not really convinced that the existing asserts are all that desirable TBH.

[lattner]

Thank you for the quick review!

mlir/lib/IR/BuiltinAttributes.cpp

265–266

I'm not sure about that in this case at least, I don't think this is a tradeoff of predictability vs convenience: if you have an attribute that might be signless, then it isn't the right thing to turn it into an APSInt and do stuff with it (because APSInt has operations that are sign specific).

This helper could default signless to unsigned or something, but any time that happened it is probably a logic bug.

That said, the getSInt() / getUInt() methods are huge footguns because they truncate to 64-bits. They should probably be removed. I don't think getUInt is even called in tree.

[rriddle]

mlir/lib/IR/BuiltinAttributes.cpp

265–266

Definitely agree for APSInt. For getSInt/getUInt, getSExtValue/getZExtValue will assert if the number of active bits > 64. These also match the similarly named methods on llvm::ConstantInt. Removing/cleaning up the asserts would also allow merging getInt into getSInt.

On usage, getSInt/getUInt aren't used that heavily inside of google, but getInt is. I chock that more up to the fact that the asserts require signed/unsigned, and signless is the most prevalent of the integer types.

mlir/include/mlir/IR/BuiltinAttributes.td

def Builtin_IntegerAttr : Builtin_Attr<"Integer"> {
   let parameters = (ins AttributeSelfTypeParameter<"">:$type, "APInt":$value);
   let builders = [
     AttrBuilderWithInferredContext<(ins "Type":$type,
                                         "const APInt &":$value), [{
       if (type.isSignlessInteger(1))
         return BoolAttr::get(type.getContext(), value.getBoolValue());
       return $_get(type.getContext(), type, value);
     }]>,
+    AttrBuilder<(ins "const APSInt &":$value), [{
+      auto signedness = value.isSigned() ?
+        IntegerType::Signed : IntegerType::Unsigned;
+      auto type = IntegerType::get($_ctxt, value.getBitWidth(), signedness);
+      return $_get(type.getContext(), type, value);
+    }]>,
     AttrBuilderWithInferredContext<(ins "Type":$type, "int64_t":$value), [{
       // `index` has a defined internal storage width.
       if (type.isIndex()) {
         APInt apValue(IndexType::kInternalStorageBitWidth, value);
         return $_get(type.getContext(), type, apValue);
       }
 
       IntegerType intTy = type.cast<IntegerType>();
let extraClassDeclaration = [{
     int64_t getInt() const;
     /// Return the integer value as a signed 64-bit int. The attribute must be
     /// a signed integer.
     int64_t getSInt() const;
     /// Return the integer value as a unsigned 64-bit int. The attribute must be
     /// an unsigned integer.
     uint64_t getUInt() const;
 
+    /// Return the value as an APSInt which carries the signed from the type of
+    /// the attribute.  This traps on signless integers types!
+    APSInt getAPSInt() const;
+
   private:
     /// Return a boolean attribute. This is a special variant of the `get`
     /// method that is used by the MLIRContext to cache the boolean IntegerAttr
     /// instances.
     static BoolAttr getBoolAttrUnchecked(IntegerType type, bool value);
 
     /// Allow access to `getBoolAttrUnchecked`.
     friend MLIRContext;

mlir/include/mlir/Support/LLVM.h

 template <typename T> class SmallVectorImpl;
 template <typename AllocatorTy> class StringSet;
 template <typename T, typename R> class StringSwitch;
 template <typename T> class TinyPtrVector;
 template <typename T, typename ResultT> class TypeSwitch;
 
 // Other common classes.
 class APInt;
+class APSInt;
 class APFloat;
 template <typename Fn> class function_ref;
 template <typename IteratorT> class iterator_range;
 class raw_ostream;
 } // end namespace llvm
 
 namespace mlir {
 // Casting operators.

 using StringSwitch = llvm::StringSwitch<T, R>;
 using llvm::TinyPtrVector;
 template <typename T, typename ResultT = void>
 using TypeSwitch = llvm::TypeSwitch<T, ResultT>;
 
 // Other common classes.
 using llvm::APFloat;
 using llvm::APInt;
+using llvm::APSInt;
 template <typename Fn> using function_ref = llvm::function_ref<Fn>;
 using llvm::iterator_range;
 using llvm::raw_ostream;
 } // namespace mlir
 
 #endif // MLIR_SUPPORT_LLVM_H

mlir/lib/IR/BuiltinAttributes.cpp

int64_t IntegerAttr::getSInt() const {
   return getValue().getSExtValue();
 }
 
 uint64_t IntegerAttr::getUInt() const {
   assert(getType().isUnsignedInteger() && "must be unsigned integer");
   return getValue().getZExtValue();
 }
 
+/// Return the value as an APSInt which carries the signed from the type of
+/// the attribute.  This traps on signless integers types!
+APSInt IntegerAttr::getAPSInt() const {
+  assert(!getType().isSignlessInteger() &&
+         "Signless integers don't carry a sign for APSInt");
+  return APSInt(getValue(), getType().isUnsignedInteger());
+}
+
 LogicalResult IntegerAttr::verify(function_ref<InFlightDiagnostic()> emitError,
                                   Type type, APInt value) {
   if (IntegerType integerType = type.dyn_cast<IntegerType>()) {
     if (integerType.getWidth() != value.getBitWidth())
       return emitError() << "integer type bit width (" << integerType.getWidth()
                          << ") doesn't match value bit width ("
                          << value.getBitWidth() << ")";
     return success();