[PATCH] D78013: [AssumeBundles] Refactor asssume builder

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

Two comments to consider but other than that LGTM.



================
Comment at: llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp:144
         FnAssume, ArrayRef<Value *>({ConstantInt::getTrue(C)}), OpBundle));
   }
 
----------------
Nit: Can you make local variables with explicit types out of the MapElem so it is easier to follow the code?


================
Comment at: llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp:193
+  Builder.InsertBeforeInstruction = I;
+  Builder.addInstruction(I);
+  if (IntrinsicInst *Intr = Builder.build()) {
----------------
I guess the above is already reason enough to have a helper to create either the `AssumeBuilderState` from an instruction or even call the build after as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78013