[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 2 05:15:21 PST 2020


lebedev.ri accepted this revision.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Looks ok to me now in principle.
I have one more question about pointer variants though (see inline)



================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14321
+  if (Args.Src->getType()->isPointerTy()) {
+    /// TODO: Use ptrmask instead of ptrtoint+gep once it is optimized well.
+    // Result = Builder.CreateIntrinsic(
----------------
I honestly wonder if (at least for `align up`) that would result in worse results,
but we'll see if/when such patch appears i guess..


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14327
+    llvm::Value* Difference = Builder.CreateSub(Result, SrcAddr, "diff");
+    Result = Builder.CreateGEP(EmitCastToVoidPtr(Args.Src), Difference,
+                               "aligned_result");
----------------
What's the semantics of these aligning builtins for pointers?
Is this GEP supposed to comply to the C17 6.5.6p8, C++ [expr.add]?
(TLDR: both the old pointer, and the newly-aligned pointer
must both point to the **same** array (allocated memory region))

I.e. can this GEP be `inbounds`? If it can, it'd be most great for optimizations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499





More information about the cfe-commits mailing list