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

[Johannes Doerfert via Phabricator via cfe-commits]

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: clang/lib/CodeGen/CodeGenFunction.cpp:2187
                                  OffsetValue, TheCheck, Assumption);
-  }
 }
 
----------------
Maybe invert the conditions:
```
llvm::Instruction *Assumption = Builder.CreateAlignmentAssumption(
      CGM.getDataLayout(), PtrValue, Alignment, OffsetValue);
 if (!SanOpts.has(SanitizerKind::Alignment))
   return;
//sanitizer stuff
```


================
Comment at: clang/test/CodeGen/builtin-align.c:48
 int up_2 = __builtin_align_up(256, 32);
-// CHECK: @up_2 = global i32 256, align 4
 
----------------
Why did these go away?


================
Comment at: llvm/test/Transforms/AlignmentFromAssumptions/simple32.ll:14
 ; CHECK-NEXT:    tail call void @llvm.assume(i1 [[MASKCOND]])
-; CHECK-NEXT:    [[TMP0:%.*]] = load i32, i32* [[A]], align 32
+; CHECK-NEXT:    [[TMP0:%.*]] = load i32, i32* [[A]], align 4
 ; CHECK-NEXT:    ret i32 [[TMP0]]
----------------
I think we should change to the new encoding here too. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71739