[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

Tyker added a comment. i believe all known issues with this patch have been fixed is it fine to reland ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71739/new/ https://reviews.llvm.org/D71739 ___

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG8d09f20798ac: [AssumeBundles] Use operand bundles to encode alignment assumptions (authored by Tyker). Repository: rG

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

Tyker updated this revision to Diff 276500. Tyker added a comment. fixed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71739/new/ https://reviews.llvm.org/D71739 Files: clang/lib/CodeGen/CodeGenFunction.cpp clang/test/CodeGen/align_value.cpp

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

Tyker updated this revision to Diff 276498. Tyker added a comment. addressed commemt. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71739/new/ https://reviews.llvm.org/D71739 Files: clang/lib/CodeGen/CodeGenFunction.cpp

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

jdoerfert accepted this revision. jdoerfert added a comment. LGTM. @lebedev.ri ? Comment at: llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp:227 } - - if (!AAPtr) -return false; - - // Sign extend the offset to 64 bits (so that it is like all of the other -

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

Tyker updated this revision to Diff 275550. Tyker added a comment. fixed the issue with multiple align in a single assume. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71739/new/ https://reviews.llvm.org/D71739 Files:

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

lebedev.ri reopened this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Reverted in 7ea46aee3670981827c04df89b2c3a1cbdc7561b . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

This revision was automatically updated to reflect the committed changes. Closed by commit rGc95ffadb2474: [AssumeBundles] Use operand bundles to encode alignment assumptions (authored by Tyker). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

Tyker updated this revision to Diff 272968. Tyker marked 2 inline comments as done. Tyker added a comment. addressed comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71739/new/ https://reviews.llvm.org/D71739 Files:

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Two nits and one question about removed check lines. If they can be re-added, LGTM. Otherwise we need to look into that. Comment at:

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

Tyker updated this revision to Diff 272194. Tyker added a comment. sorry for not doing much recently. i split with an NFC patch with only test updates. and addressed comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71739/new/

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

jdoerfert added a comment. I think this should go on, could you address the minor comments so we can merge it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71739/new/ https://reviews.llvm.org/D71739

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

jdoerfert added a comment. I think the code looks good, we should make the test changes clearer, see below. Comment at: clang/test/CodeGen/align_value.cpp:7 double & z __attribute__((align_value(128 { }; -// CHECK: define void @_Z3fooPdS_Rd(double* align 64 %x,

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

rjmccall added a comment. > there is any IR legality restriction about what combination or number of > bundles can be present in an assume. however there is restrictions about > argument of a given bundle. > having a single "align" bundle is just how the front-end used assume bundles > for

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

Tyker added a comment. In D71739#2048409 , @rjmccall wrote: > Is there a good reason for this to use the same `llvm.assume` intrinsic as > before? thee hasn't been any discussion i am aware of on this topic. but personally i think keeping the same

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

rjmccall added a comment. Is there a good reason for this to use the same `llvm.assume` intrinsic as before? Are there restrictions about what assumptions can be combined on a single intrinsic call? There can only be one bundle of a particular name on an instruction, right?

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

jdoerfert added a comment. I guess alignment is one of the main users of the old format, great to see it go :) First set of comments. Comment at: llvm/lib/Analysis/AssumeBundleQueries.cpp:104 +return 1; + }; if (BOI.End - BOI.Begin > ABA_Argument) I