TIFitis marked 2 inline comments as done.
TIFitis added inline comments.
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4064-4077
+ Value *BaseGEPPtr = Builder.CreateInBoundsGEP(
+ ArrI8PtrTy, MapperAllocas.ArgsBase,
+ {Builder.getInt32(0), Builder.getInt32(Index)});
+ Value *BaseCastPtr = Builder.CreateBitCast(
+ BaseGEPPtr, MapOpPtrBase->getType()->getPointerTo());
+ Builder.CreateStore(MapOpPtrBase, BaseCastPtr);
+ MapInfo.BasePtr = BaseGEPPtr;
----------------
I think these should just use the opaque ptr type instead of the ArrI8PtrTy it
is currently using, and we can also get rid of the BitCast instructions.
However doing so would break the tests in OpenMPIRBuilderTest.cpp as that unit
test still uses typed pointers.
AFAIK the consensus is to completely get rid of typed pointers in llvm 17.
Should I updated this code to use opaque pointers and let the test fail until
it gets converted to opaque as well?
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4261
+ bool Temp;
+ return Val->getPointerDereferenceableBytes(M.getDataLayout(), Temp, Temp);
}
----------------
jdoerfert wrote:
> TIFitis wrote:
> > `getPointerDereferenceableBytes` doesn't fully support `Arguments` and
> > mostly returns 0.
> >
> > I am looking at adding support for arguments in a separate patch, in the
> > meantime the tests added in this patch fail because of incorrect
> > @.offload_sizes.
> That function is not the right one for code generation. The sizes need to
> come from the type + data layout, and potentially the expression used in the
> map.
Thanks for letting me know.
Just to clarify, do you mean it should come from the MLIR::Type?
The llvm::Type doesn't let me get much information because of Opaque pointers.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146557/new/
https://reviews.llvm.org/D146557
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
