Skip to content

Conversation

@koparasy
Copy link
Contributor

No description provided.

rhs = Builder.createAddrSpaceCast(rhs, lhsPtrTy);
} else if (lhsPtrTy != rhsPtrTy) {
// Same addrspace but different pointee/type → bitcast is fine
rhs = Builder.createBitcast(rhs, lhsPtrTy);
Copy link

@mahmood82 mahmood82 Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, Can you please try createPtrBitcast

I have a local patch for that function to allow passing AS, I'll upload it ASAP:

----------- clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h -----------
index b7b599e1289e..fd096ff97eb7 100644
@@ -574,7 +574,7 @@ public:
 
   mlir::Value createPtrBitcast(mlir::Value src, mlir::Type newPointeeTy) {
     assert(mlir::isa<cir::PointerType>(src.getType()) && "expected ptr src");
-    return createBitcast(src, getPointerTo(newPointeeTy));
+    return createBitcast(src, getPointerTo(newPointeeTy, mlir::cast<cir::PointerType>(src.getType()).getAddrSpace()));
   }

In addition I have another patch to fix the case of different AS and different data type. I'll upload them ASAP and let you know.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to use createPtrBitcast. When you patch lands and fixes AS I will be happy to revise.

Copy link
Contributor Author

@koparasy koparasy Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion! I took a closer look at createPtrBitcast, and in this particular spot I already have the full destination pointer type (lhsPtrTy) available, including the address space information. Using createBitcast(rhs, lhsPtrTy) lets me preserve that exact type without reconstructing it.

createPtrBitcast is super useful in places where we only know the new pointee type, but here we already computed the exact pointer type we want to cast to, so sticking to createBitcast keeps the intent clearer.

Happy to switch if we move toward a uniform helper style later, but for this case preserving the full cir::PointerType seems cleaner.

Thanks again for taking a look — really appreciate the review!


mlir::Type lhsTy = lhs.getType();
mlir::Type rhsTy = rhs.getType();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should follow OG skeleton here, similar names if possible would be helpful:

// Okay, figure out the element size.
  const BinaryOperator *expr = cast<BinaryOperator>(op.E);
  QualType elementType = expr->getLHS()->getType()->getPointeeType();
 ...
  // For a variable-length array, this is going to be non-constant.
  if (const VariableArrayType *vla
        = CGF.getContext().getAsVariableArrayType(elementType)) {
    llvm_unrecheable("NYI");
...
  else {
    ...
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants