Missing getelementptr operation in the LLVM dialect

[fuzzypixelz]

Expected Behavior

The operation in question would be generated (alongside a pattern).

Actual Behavior

Only the documentation is present.

Steps to Reproduce the Problem

1. Check https://google.github.io/mlir-hs/mlir-hs-0.1.0.0/MLIR-AST-Dialect-LLVM.html#g:58

Comments

For a couple missing operations in mlir-hs, the solution was adding missing Attributes in the Haskell side, but in this case I'm not very sure why hs-generators.cc refuses to generate it.

[fuzzypixelz]

Turns out you can pass -explain-mising to hs-generators.cc from Setup.hs. It's missing the I32ElementsAttr attribute!

[jpienaar]

You'll probably run into #36 post enabling it, it is easy to skip generating the test for these ops but I didn't check if this works past this then.

[fuzzypixelz]

I did not run the tests but I surely ran into issues when generating code that uses GEP.

The generated builder (the pattern fails to generate, did not investigate why exactly that is, but it complained about the variable size of operands?).

pattern InternalGEPOpAttributes :: () => (Ix a0, Show a0) => (AST.IStorableArray a0 Int32) -> NamedAttributes
pattern InternalGEPOpAttributes structIndices <- ((\m -> (M.lookup "structIndices" m)) -> (Just (DenseElementsAttr (IntegerType Signless 32) (DenseInt32 structIndices))))
  where InternalGEPOpAttributes structIndices = M.fromList $ [("structIndices", DenseElementsAttr (IntegerType Signless 32) (DenseInt32 structIndices))]

-- | A builder for @llvm.getelementptr@.
getelementptr :: (Ix a0, Show a0) => MonadBlockBuilder m => Type -> Value -> [Value] -> (AST.IStorableArray a0 Int32) -> m Value
getelementptr ty0 base indices structIndices  = do
  Control.Monad.liftM Prelude.head (AST.emitOp (Operation
          { opName = "llvm.getelementptr"
          , opLocation = UnknownLocation
          , opResultTypes = Explicit [ty0]
          , opOperands = ([(AST.operand base)] ++ (AST.operands indices))
          , opRegions = []
          , opSuccessors = []
          , opAttributes = (InternalGEPOpAttributes structIndices)
          }))

The structIndices attributes winds up being DenseElementsAttr (IntegerType Signless 32) (DenseInt32 structIndices). The however triggered a casting assert in LLVM to fail on my end, so I rectified it to be:

DenseElementsAttr
                        (VectorType [length structIndices]
                                    (IntegerType Signless 32)
                        )
                        ( DenseInt32
                        $ listArray (1, length structIndices) structIndices
                        )

Where this time structIndices is a regular list and not an IStorableArray (more convenient?). But the real change here is the VectorType

This can be traced to line ~227 of hs-generators.cc:

      {"I64ElementsAttr", {"DenseElementsAttr (IntegerType Signless 64) (DenseInt64 {0})",
                           "(AST.IStorableArray {0} Int64)", {"Ix {0}", "Show {0}"}, {"PatternUtil.DummyIx"}}},
      {"I32ElementsAttr", {"DenseElementsAttr (IntegerType Signless 32) (DenseInt32 {0})",
                           "(AST.IStorableArray {0} Int32)", {"Ix {0}", "Show {0}"}, {"PatternUtil.DummyIx"}}},

(the second line is one I added myself by copying the I64 version :^)) So I think IntegerType Signless X) is to be replaced with VectorType [Prelude.length {0}] (IntegerType Signless X)

That formatting might be wrong since I'm not familiar with how it works.

[apaszke]

That's weird. While this might be a good fix for getelementptr, I don't think that it's a change we should make in general. It just seems that I32ElementsAttr doesn't carry enough information, meaning that we're unable to choose a universally good variant here. If we see 5 integers, are they a dense array of 5 ints, or are they a 1-sized array containing a 5-sized vector of ints? They're both, but we have to choose one! Seems like there might be another kind of attribute type that should be added on the MLIR side to disambiguate situations like this one.

[jpienaar]

I32ElementsAttr's type in MLIR is a Tensor of rank 0 with i32 (https://github.com/llvm/llvm-project/blob/45428412fd7c9900d3d6ac9803aa7dcf6adfa6fe/mlir/include/mlir/IR/OpBase.td#L1542), but that's only the ODS definition (that should apply locally here during op builder generation). There is the more general case of DenseElementsAttr where the type and values would be needed indeed.