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

This revision was automatically updated to reflect the committed changes. Closed by commit rG8c387cbea76b: Add builtins for aligning and checking alignment of pointers and integers (authored by arichardson). Changed prior to commit: https://reviews.llvm.org/D71499?vs=236243&id=237190#toc Repos

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

erichkeane added a comment. Sorry, I didn't realize you were waiting for me. This seems fine as it is, I don't see the attribute buying us anything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71499/new/ https://reviews.llvm.org/D71499 _

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

arichardson added a comment. In D71499#1810526 , @lebedev.ri wrote: > Land this? (branching is near..) I was planning to wait until Sunday for @erichkeane to comment whether the AST attribute is needed and if I don't hear back until then land I would co

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

lebedev.ri added a comment. Land this? (branching is near..) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71499/new/ https://reviews.llvm.org/D71499 ___ cfe-commits mailing list cfe-commits@lists.llvm

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

lebedev.ri accepted this revision. lebedev.ri added a subscriber: erichkeane. lebedev.ri added a comment. This revision is now accepted and ready to land. Nice. Thank you for working on this. I think this finally fully looks good to me. Not sure whether we should be really adding an attribute to

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

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61248 tests passed, 0 failed and 736 were skipped. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

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

arichardson updated this revision to Diff 236243. arichardson added a comment. - Add a test that the new alignment is propagated to e.g. llvm.memcpy To do this emit a llvm.assume for pointer types and mark the builtin as having an alloc_align attribute (although that does not seem to do anything)

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

nlopes added a comment. @lebedev.ri I agree with you that the semantics of these alignment builtins should only return a pointer that is of the same object as the one given as input. Otherwise, these builtins would be even worst that ptr2int/int2ptr, since their result could alias with any othe

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

lebedev.ri added a comment. In D71499#1802134 , @lebedev.ri wrote: > For future reference, if anyone needs here's the C versions of these builtins: > https://godbolt.org/z/oHeAh- > > ^ looks like we are missing some backend-level folds for round-down vari

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

lebedev.ri added a comment. For future reference, if anyone needs here's the C versions of these builtins: https://godbolt.org/z/oHeAh- ^ looks like we are missing some backend-level folds for round-down variant, filed https://bugs.llvm.org/show_bug.cgi?id=8 Repository: rG LLVM Github Mo

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

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61176 tests passed, 0 failed and 729 were skipped. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

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

lebedev.ri added a comment. Hmm, i keep coming up with things here, sorry :/ I think we should take one more step - explicitly call out that the result of `__builtin_align_*` is, well, aligned :) For that, we need to add `__attribute__((alloc_align(2)))` attribute. Despite the name, it does not

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

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61176 tests passed, 0 failed and 729 were skipped. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

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

arichardson updated this revision to Diff 235903. arichardson added a comment. - Fix typos caused by dodgy n key on my keyboard. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71499/new/ https://reviews.llvm.org/D71499 Files: clang/docs/LanguageE

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

arichardson added a comment. In D71499#1801421 , @lebedev.ri wrote: > (temporarily undoing my review since it looks like things may change) > > In D71499#1801302 , @arichardson > wrote: > > > ... > > > Aha! :) > >

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

arichardson updated this revision to Diff 235899. arichardson marked 2 inline comments as done. arichardson added a comment. - Generate inbounds GEP. Also mention that values must point to the same allocation in the documentation. - Clarify that correctly aligned values do not change. Repositor

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

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. (temporarily undoing my review since it looks like things may change) In D71499#1801302 , @arichardson wrote: > ... Aha! :) In D71

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

jrtc27 added inline comments. Comment at: clang/docs/LanguageExtensions.rst:2844 +The builtins ``__builtin_align_up``, ``__builtin_align_down``, return their +first argument aligned up/down to the next multiple of the second argument. +The builtin ``__builtin_is_aligned`` returns

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

arichardson added a subscriber: brooks. arichardson added a comment. > What i'm asking is: > > - Are these builtins designed (as per `clang/docs/LanguageExtensions.rst`) to > only be passed pointers in-bounds to the allocated memory chunk (`Logical > pointer`*), or any random bag of bits casted

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

lebedev.ri added reviewers: nlopes, aqjune. lebedev.ri added a subscriber: nlopes. lebedev.ri added a comment. (would be good for @nlopes to comment, maybe i'm overweighting this..) In D71499#1801119 , @arichardson wrote: > In D71499#1801104

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

arichardson added a comment. In D71499#1801104 , @lebedev.ri wrote: > Looks ok to me now in principle. > I have one more question about pointer variants though (see inline) I am not sure the GEP can be inbounds since I have seen some cases where aligni

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

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/li

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

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61172 tests passed, 0 failed and 728 were skipped. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

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

arichardson updated this revision to Diff 235848. arichardson marked an inline comment as done. arichardson added a comment. Improved code generation with a single GEP The pointer case of align_up is now generates the expected add+mask assembly even without the llvm.ptrmask intrinsic. Reposito

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

lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14318 + } + // Negate the mask to only clear the lower bits. + llvm::Value *Result; But this isn't what we are doing, negation != inversion. Repository: rG LLVM Github Monorepo

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

lebedev.ri added a comment. In D71499#1800636 , @arichardson wrote: > Address feedback: Avoid inttoptr by using ptrtoint + GEP instead. > > Using two GEPs for align_up seems to generate better code than select + a > single GEP. > See https://godbolt.org/

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

merge_guards_bot added a comment. {icon times-circle color=red} Unit tests: fail. 61172 tests passed, 1 failed and 728 were skipped. failed: Clang.CodeGen/builtin-align-array.c {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

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

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61173 tests passed, 0 failed and 728 were skipped. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

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

arichardson updated this revision to Diff 235798. arichardson added a comment. Address feedback: Avoid inttoptr by using ptrtoint + GEP instead. Using two GEPs for align_up seems to generate better code than select + a single GEP. See https://godbolt.org/z/UdPjZk Repository: rG LLVM Github M

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

arichardson marked an inline comment as done. arichardson added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14322 + if (SrcForMask->getType()->isPointerTy()) { +/// TODO: Use ptrmask instead of ptrtoint/inttoptr +// Result = Builder.CreateIntrinsic(

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

arichardson updated this revision to Diff 235799. arichardson added a comment. - Also update the other codegen test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71499/new/ https://reviews.llvm.org/D71499 Files: clang/docs/LanguageExtensions.rst

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

lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14322 + if (SrcForMask->getType()->isPointerTy()) { +/// TODO: Use ptrmask instead of ptrtoint/inttoptr +// Result = Builder.CreateIntrinsic( Until we are there, can we still e

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

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61172 tests passed, 0 failed and 728 were skipped. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

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

arichardson updated this revision to Diff 235764. arichardson marked an inline comment as done. arichardson added a comment. - Address remaining comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71499/new/ https://reviews.llvm.org/D71499 File

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a few nits to be fixed. Comment at: clang/lib/AST/ExprConstant.cpp:8156-8158 + } else if (const Expr *E = Value.Base.dyn_cast()) { +return GetAli

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

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61165 tests passed, 0 failed and 728 were skipped. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

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

arichardson updated this revision to Diff 235724. arichardson marked 9 inline comments as done. arichardson added a comment. - Address feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71499/new/ https://reviews.llvm.org/D71499 Files: clang

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

aaron.ballman added inline comments. Comment at: clang/docs/LanguageExtensions.rst:2844 +The builtins ``__builtin_align_up``, ``__builtin_align_down``, return their +first argument aligned up/down to the next multiple of the second argument. +The builtin ``__builtin_is_aligned``

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

arichardson added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71499/new/ https://reviews.llvm.org/D71499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.or

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

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61055 tests passed, 0 failed and 728 were skipped. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

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

arichardson updated this revision to Diff 234921. arichardson added a comment. - Add documentation for the new alignment builtins Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71499/new/ https://reviews.llvm.org/D71499 Files: clang/docs/Language

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

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 60963 tests passed, 0 failed and 726 were skipped. {icon times-circle color=red} clang-format: fail. Please format your changes with clang-format by running `git-clang-format HEAD^` or apply this patch

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

arichardson updated this revision to Diff 234064. arichardson added a comment. Fix align32array[0] -> &align32array[0] typo in test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71499/new/ https://reviews.llvm.org/D71499 Files: clang/include/cla

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

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 60961 tests passed, 0 failed and 726 were skipped. {icon times-circle color=red} clang-format: fail. Please format your changes with clang-format by running `git-clang-format HEAD^` or apply this patch

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

arichardson updated this revision to Diff 234051. arichardson marked 6 inline comments as done. arichardson added a comment. - Add support for constant-evaluating pointer values - Add errors messages for constant evaluation - Extend tests for constant evaluation - Use ptrtoint+inttoptr instead of

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

theraven added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14323 +Result = Builder.CreateIntrinsic( +Intrinsic::ptrmask, {Args.SrcType, SrcForMask->getType(), Args.IntType}, +{SrcForMask, NegatedMask}, nullptr, "aligned_result");

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

fhahn added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14323 +Result = Builder.CreateIntrinsic( +Intrinsic::ptrmask, {Args.SrcType, SrcForMask->getType(), Args.IntType}, +{SrcForMask, NegatedMask}, nullptr, "aligned_result"); ---

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

rsmith added a comment. To me these seem sufficiently useful to be worth having. Please add documentation for them. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2913 +def warn_alignment_builtin_useless : Warning< + "%select{aligning a value|checking whether a

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

lebedev.ri marked an inline comment as done. lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14323 +Result = Builder.CreateIntrinsic( +Intrinsic::ptrmask, {Args.SrcType, SrcForMask->getType(), Args.IntType}, +{SrcForMask, Negated

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

arichardson marked an inline comment as done. arichardson added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14323 +Result = Builder.CreateIntrinsic( +Intrinsic::ptrmask, {Args.SrcType, SrcForMask->getType(), Args.IntType}, +{SrcForMask, Negat

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

lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14323 +Result = Builder.CreateIntrinsic( +Intrinsic::ptrmask, {Args.SrcType, SrcForMask->getType(), Args.IntType}, +{SrcForMask, NegatedMask}, nullptr, "aligned_result"); --

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

merge_guards_bot added a comment. {icon times-circle color=red} Unit tests: fail. 60874 tests passed, 1 failed and 726 were skipped. failed: Clang.SemaCXX/builtin-align-cxx.cpp {icon check-circle color=green} clang-format: pass. Build artifacts

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

arichardson created this revision. arichardson added reviewers: rsmith, aaron.ballman, theraven, fhahn. Herald added a project: clang. Herald added a subscriber: cfe-commits. This change introduces three new builtins (which work on both pointers and integers) that can be used instead of common bit