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

[Johannes Doerfert via Phabricator via cfe-commits]

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, double* align 32 %y, double* align 128 dereferenceable(8) %z)
 
----------------
Can you first make an NFC patch introducing autogenerated check lines for the tests, with `--function-signature` please. That way the difference becomes clear here.

I don't think such a patch is controversial but we should give others the chance to comment on it. 


================
Comment at: llvm/lib/IR/IRBuilder.cpp:1084
+                                                         Value *OffsetValue) {
+  SmallVector<Value *, 2> Vals({PtrValue, AlignValue});
+  if (OffsetValue)
----------------
Nit: Make it size 4 to avoid a trivial dynamic allocation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71739