[mlir] Simplify a few cast implementations

[rkayaith]

Summary

{Attribute,Type}::classof are never actually called at runtime due to
the early exit in their CastInfo implementations. By using `if
constexpr` we can avoid needing to define them.

We also don't need to check is_same_v here, since this is already
covered by is_base_of_v.

[rkayaith]

mlir/include/mlir/IR/Attributes.h

394–398

On second look I'm not sure why is_same_v is used here, since is_base_of_v<T, T> is already true for the types we care about: https://en.cppreference.com/w/cpp/types/is_base_of

[rkayaith]

remove is_same_v check

[AlexEichenberger]

FYI, we get a lot of

/workdir/llvm-project/mlir/include/mlir/IR/Attributes.h:391:49: warning: parameter 'ty' set but not used [-Wunused-but-set-parameter]
  391 |   static inline bool isPossible(mlir::Attribute ty) {

warning as it appears that ty is not seen as used. Would it be possible to add a fake use?
Apologies as it may be compiler specific

[rkayaith]

In D142863#4127655, @AlexEichenberger wrote:

FYI, we get a lot of

/workdir/llvm-project/mlir/include/mlir/IR/Attributes.h:391:49: warning: parameter 'ty' set but not used [-Wunused-but-set-parameter]
  391 |   static inline bool isPossible(mlir::Attribute ty) {

warning as it appears that ty is not seen as used. Would it be possible to add a fake use?
Apologies as it may be compiler specific

Which compiler are you using? It's a bit unfortunate it warns here. I'm not sure exactly what the policy is for warnings, but if your compiler falls under LLVM's supported compilers (https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library) then it seems reasonable to me to work around it.

[AlexEichenberger]

Here is what our virtual machine is using:

--   System                    : Linux
--   C++ compiler              : /usr/bin/c++
--   C++ compiler version      : 9.3.0

and

root@1250b049ac2b:~/workspace/atests/ops# /usr/bin/c++ -v
Using built-in specs.
COLLECT_GCC=/usr/bin/c++
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/9/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:hsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 9.3.0-17ubuntu1~20.04' --with-bugurl=file:///usr/share/doc/gcc-9/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++,gm2 --prefix=/usr --with-gcc-major-version-only --program-suffix=-9 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none=/build/gcc-9-HskZEa/gcc-9-9.3.0/debian/tmp-nvptx/usr,hsa --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 9.3.0 (Ubuntu 9.3.0-17ubuntu1~20.04)

I am not sure how we arrived at this compiler, I can check if that can be changed or not. It is what we use to deliver onnx-mlir on our x86 / Power / Z linux machines at IBM.

[AlexEichenberger]

I see from your convenient link that our gcc compiler 9.3.0 which is more recent than the minimum recommendation of LLVM which is 7.1

[rkayaith]

In D142863#4129071, @AlexEichenberger wrote:

I see from your convenient link that our gcc compiler 9.3.0 which is more recent than the minimum recommendation of LLVM which is 7.1

I pushed 82ae83a8487df0747195d1abbda11920fda11eb8 to silence these.

mlir/include/mlir/IR/Attributes.h

public:
   bool isa_and_nonnull() const;
   template <typename U>
   U dyn_cast() const;
   template <typename U>
   U dyn_cast_or_null() const;
   template <typename U>
   U cast() const;
 
-  // Support dyn_cast'ing Attribute to itself.
-  static bool classof(Attribute) { return true; }
-
   /// Return a unique identifier for the concrete attribute type. This is used
   /// to support dynamic type casting.
   TypeID getTypeID() { return impl->getAbstractAttribute().getTypeID(); }
 
   /// Return the context this attribute belongs to.
   MLIRContext *getContext() const;
 
   /// Get the dialect this attribute is registered to.
struct CastInfo<To, From,
   /// children, the val.getTypeID() inside T::classof will use the static
   /// getTypeID of the parent instead of the non-static Type::getTypeID that
   /// returns the dynamic ID. This means that T::classof would end up comparing
   /// the static TypeID of the children to the static TypeID of its parent,
   /// making it impossible to downcast from the parent to the child.
   static inline bool isPossible(mlir::Attribute ty) {
     /// Return a constant true instead of a dynamic true when casting to self or
     /// up the hierarchy.
-    return std::is_same_v<To, std::remove_const_t<From>> ||
-           std::is_base_of_v<To, From> || To::classof(ty);
+    if constexpr (std::is_base_of_v<To, From>) {
+      return true;
+    } else {
+      return To::classof(ty);
+    }
   }
   static inline To doCast(mlir::Attribute attr) { return To(attr.getImpl()); }
 };
 
 } // namespace llvm
 
 #endif

mlir/include/mlir/IR/Types.h

public:
   bool isa_and_nonnull() const;
   template <typename U>
   U dyn_cast() const;
   template <typename U>
   U dyn_cast_or_null() const;
   template <typename U>
   U cast() const;
 
-  // Support type casting Type to itself.
-  static bool classof(Type) { return true; }
-
   /// Return a unique identifier for the concrete type. This is used to support
   /// dynamic type casting.
   TypeID getTypeID() { return impl->getAbstractType().getTypeID(); }
 
   /// Return the MLIRContext in which this type was uniqued.
   MLIRContext *getContext() const;
 
   /// Get the dialect this type is registered to.
struct CastInfo<
   /// the val.getTypeID() inside T::classof will use the static getTypeID of the
   /// parent instead of the non-static Type::getTypeID that returns the dynamic
   /// ID. This means that T::classof would end up comparing the static TypeID of
   /// the children to the static TypeID of its parent, making it impossible to
   /// downcast from the parent to the child.
   static inline bool isPossible(mlir::Type ty) {
     /// Return a constant true instead of a dynamic true when casting to self or
     /// up the hierarchy.
-    return std::is_same_v<To, std::remove_const_t<From>> ||
-           std::is_base_of_v<To, From> || To::classof(ty);
+    if constexpr (std::is_base_of_v<To, From>) {
+      return true;
+    } else {
+      return To::classof(ty);
+    };
   }
   static inline To doCast(mlir::Type ty) { return To(ty.getImpl()); }
 };
 
 } // namespace llvm
 
 #endif // MLIR_IR_TYPES_H

mlir/include/mlir/IR/Value.h

struct CastInfo<
   /// children, the val.getKind() inside T::classof will use the static
   /// getKind() of the parent instead of the non-static ValueImpl::getKind()
   /// that returns the dynamic type. This means that T::classof would end up
   /// comparing the static Kind of the children to the static Kind of its
   /// parent, making it impossible to downcast from the parent to the child.
   static inline bool isPossible(mlir::Value ty) {
     /// Return a constant true instead of a dynamic true when casting to self or
     /// up the hierarchy.
-    return std::is_same_v<To, std::remove_const_t<From>> ||
-           std::is_base_of_v<To, From> || To::classof(ty);
+    if constexpr (std::is_base_of_v<To, From>) {
+      return true;
+    } else {
+      return To::classof(ty);
+    }
   }
   static inline To doCast(mlir::Value value) { return To(value.getImpl()); }
 };
 
 } // namespace llvm
 
 #endif