[PATCH] D111566: [SYCL] Fix function pointer address space

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 10 13:58:24 PST 2022


rjmccall added inline comments.


================
Comment at: clang/lib/AST/ASTContext.cpp:11494
+  const Type *TypePtr = T.getTypePtr();
+  return TypePtr->isFunctionType()
+             ? llvm::DataLayout(getTargetInfo().getDataLayoutString())
----------------
You can just do `T->isFunctionType()`.


================
Comment at: clang/lib/AST/ASTContext.cpp:11497
+                   .getProgramAddressSpace()
+             : getTargetAddressSpace(T.getQualifiers());
+}
----------------
If a function type has an address space qualifier, we should prefer that, right?  Or is that impossible by construction?


================
Comment at: clang/lib/CodeGen/CodeGenTypes.cpp:747
                                   : ConvertTypeForMem(FTy);
-    unsigned AS = Context.getTargetAddressSpace(FTy);
+    unsigned AS = Context.getTargetAddressSpace(FTy.getQualifiers());
     ResultType = llvm::PointerType::get(PointeeType, AS);
----------------
Please add a comment here explaining that it's important to not just use `FTy` because in the absence of an explicit address space we need to get the default address space for a data pointer, not a function.


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

https://reviews.llvm.org/D111566



More information about the cfe-commits mailing list