[PATCH] D113755: [Flang] Add the FIR LLVMPointer Type

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added inline comments.


================
Comment at: flang/lib/Optimizer/Dialect/FIRType.cpp:475
+//===----------------------------------------------------------------------===//
+// LLVMPointerType
+//===----------------------------------------------------------------------===//
----------------
clementval wrote:
> kiranchandramohan wrote:
> > kiranchandramohan wrote:
> > > kiranchandramohan wrote:
> > > > Mogball wrote:
> > > > > Mogball wrote:
> > > > > > kiranchandramohan wrote:
> > > > > > > clementval wrote:
> > > > > > > > nit: Can we do this with assembly format directly?
> > > > > > > I gave this a try but was not successful. It could be because I used it incorrectly or because the support for all types is not yet available.  @Mogball ?
> > > > > > > 
> > > > > > > Specifically, I made the following change to the definition of the type.
> > > > > > > 
> > > > > > > 
> > > > > > > ```
> > > > > > >   let parameters = (ins "mlir::Type":$eleTy);
> > > > > > > 
> > > > > > > + let assemblyFormat = "`<` $eleTy `>`";
> > > > > > > ```
> > > > > > > 
> > > > > > > I got the following errors.
> > > > > > > ```
> > > > > > > tools/flang/include/flang/Optimizer/Dialect/FIROpsTypes.cpp.inc: In static member function ‘static mlir::Type fir::LLVMPointerType::parse(mlir::AsmParser&)’:
> > > > > > > tools/flang/include/flang/Optimizer/Dialect/FIROpsTypes.cpp.inc:569:52: error: incomplete type ‘mlir::FieldParser<mlir::Type>’ used in nested name specifier
> > > > > > >   569 |   _result_eleTy = ::mlir::FieldParser<mlir::Type>::parse(parser);
> > > > > > >       |                                                    ^~~~~
> > > > > > > tools/flang/include/flang/Optimizer/Dialect/FIROpsTypes.cpp.inc:578:29: error: no matching function for call to ‘fir::LLVMPointerType::get(mlir::MLIRContext*, mlir::Type&)’
> > > > > > >   578 |     _result_eleTy.getValue());
> > > > > > >       |                             ^
> > > > > > > tools/flang/include/flang/Optimizer/Dialect/FIROpsTypes.cpp.inc:554:17: note: candidate: ‘static fir::LLVMPointerType fir::LLVMPointerType::get(mlir::Type)’
> > > > > > >   554 | LLVMPointerType LLVMPointerType::get(mlir::Type elementType) {
> > > > > > > ```
> > > > > > I forgot to add a template specialization for Types. Let me get that right now.
> > > > > https://reviews.llvm.org/D113867
> > > > Thanks, @Mogball that fixes the fieldparser issue. But I still get one more error about the `get` Function. Would you know what causes this?
> > > > 
> > > > ```
> > > > tools/flang/include/flang/Optimizer/Dialect/FIROpsTypes.cpp.inc: In static member function ‘static mlir::Type fir::LLVMPointerType::parse(mlir::AsmParser&)’:
> > > > tools/flang/include/flang/Optimizer/Dialect/FIROpsTypes.cpp.inc:578:29: error: no matching function for call to ‘fir::LLVMPointerType::get(mlir::MLIRContext*, mlir::Type&)’
> > > >   578 |     _result_eleTy.getValue());
> > > >       |                             ^
> > > > tools/flang/include/flang/Optimizer/Dialect/FIROpsTypes.cpp.inc:554:17: note: candidate: ‘static fir::LLVMPointerType fir::LLVMPointerType::get(mlir::Type)’
> > > >   554 | LLVMPointerType LLVMPointerType::get(mlir::Type elementType) {
> > > >       |                 ^~~~~~~~~~~~~~~
> > > > tools/flang/include/flang/Optimizer/Dialect/FIROpsTypes.cpp.inc:554:17: note:   candidate expects 1 argument, 2 provided
> > > > ```
> > > > 
> > > > If I comment out the additional argument (the context) then the error goes away.
> > > Please ignore, I think this is due to the builder being the following,
> > > 
> > > 
> > > ```
> > >  let builders = [
> > >     TypeBuilderWithInferredContext<(ins "mlir::Type":$elementType), [{
> > >       return Base::get(elementType.getContext(), elementType);
> > >     }]>,
> > >   ];
> > > ```
> > @clementval Done in flang/include/flang/Optimizer/Dialect/FIRTypes.td.
> Thanks! can you just remove the two functions below before landing this?
The functions below are for the FIR pointer type. I have not added the functions for the FIR LLVM Pointer type.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113755/new/

https://reviews.llvm.org/D113755