[PATCH] D68746: [Clang][OpenMP Offload] Move offload registration code to the wrapper

Sergey Dmitriev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 11 08:51:34 PDT 2019


sdmitriev marked an inline comment as done.
sdmitriev added inline comments.


================
Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:74
+  IntegerType *getSizeTTy() {
+    switch (M.getDataLayout().getPointerTypeSize(Type::getInt8PtrTy(C))) {
+    case 4u:
----------------
grokos wrote:
> sdmitriev wrote:
> > ABataev wrote:
> > > sdmitriev wrote:
> > > > ABataev wrote:
> > > > > Same question as before: maybe better to make the size of size_t type a parameter of a tool?
> > > > As I remember you also had another suggestion - change size_t to intptr_t. That will eliminate the need to an additional parameter for size type. Will it be better?
> > > In thi case we'll need to change the type in the libomptarget.
> > Right. @grokos , do you see any potential problems in changing __tgt_offload_entry::size type from size_t to intptr_t?
> As long as `intptr_t` has the same size as `size_t` it should be fine. Of course, if this is not the case, then if libomptarget tries to load an older image where `sizeof(__tgt_offload_entry::size) != sizeof(intptr_t)` then backwards compatibility will have been broken. Fortunately, on all platforms supported by released versions of libomptarget so far (x86_64, ppc64, aarch64) if I'm not mistaken `sizeof(size_t) == sizeof(initptr_t)`, so I don't think we'll break anything.
@ABataev, as I understand clang CG assumes that intptr_t, size_t, and ptrdiff_t are the same types.
Please look at clang/lib/CodeGen/CodeGenTypeCache.h, lines 44-49

```
  /// intptr_t, size_t, and ptrdiff_t, which we assume are the same size.
  union {
    llvm::IntegerType *IntPtrTy;
    llvm::IntegerType *SizeTy;
    llvm::IntegerType *PtrDiffTy;
  };
```
So, I guess what we have here is Ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68746





More information about the cfe-commits mailing list