[libcxx-commits] [PATCH] D158861: [llvm] Move CallInst::CreateMalloc to IRBuilderBase::CreateMalloc

[Nikita Popov via Phabricator via libcxx-commits]

nikic added a comment.

The libcxxabi/test/test_demangle.pass.cpp file should not be modified. It uses LLVM symbol names but is completely unrelated.



================
Comment at: llvm/bindings/ocaml/llvm/llvm.mli:2038
-    See the method [llvm::CallInst::CreateMalloc]. *)
-val build_malloc : lltype -> string -> llbuilder -> llvalue
-
----------------
kwk wrote:
> @nikic I wonder if I should have kept this. I don't know no *.mli files if this was a binding to CallInst::CreateMalloc, then it should go I think.
Yes, this should be kept. You can just adjust the method name in the comment.


================
Comment at: llvm/examples/BrainF/BrainF.cpp:99
+                                  "arr");
   cast<Instruction>(ptr_arr)->insertInto(BB, BB->end());
 
----------------
I think you need to drop this line, as CreateMalloc already does the insertion.


================
Comment at: llvm/include/llvm/IR/IRBuilder.h:633
+                         Value *ArraySize, Function *MallocF,
+                         const Twine &Name);
+
----------------
We usually do not require a name in IRBuilder.


================
Comment at: llvm/lib/IR/Core.cpp:3539
+      unwrap(B)->CreateMalloc(ITy, unwrap(Ty), AllocSize, nullptr, nullptr, "");
   return wrap(unwrap(B)->Insert(Malloc, Twine(Name)));
 }
----------------
This should directly return the CreateMalloc result, as that already performs the insertion. Also pass Name to it.


================
Comment at: llvm/lib/IR/IRBuilder.cpp:308
+  // malloc(type, arraySize) becomes:
+  //       bitcast (i8* malloc(typeSize*arraySize)) to type*
+  if (!ArraySize)
----------------
Drop the bitcasts from this comment, as they're no longer present.


================
Comment at: llvm/lib/IR/IRBuilder.cpp:325
+      // Multiply type size by the array size...
+      AllocSize = BinaryOperator::CreateMul(ArraySize, AllocSize, "mallocsize");
+    }
----------------
Call IRBuilder's `CreateBinOp()` here, to actually insert the instruction.


================
Comment at: llvm/lib/IR/IRBuilder.cpp:332
+  Module *M = BB->getParent()->getParent();
+  Type *BPTy = PointerType::getUnqual(BB->getContext());
+  FunctionCallee MallocFunc = MallocF;
----------------
Use the `Context` member.


================
Comment at: llvm/lib/IR/IRBuilder.cpp:339
+
+  MCall = CallInst::Create(MallocFunc, AllocSize, OpB, "MCall");
+
----------------
Use IRBuilder's `CreateCall()` to also insert the instruction.

Also pass `Name` instead of `"MCall"` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158861