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

[Nikita Popov via Phabricator via libcxx-commits]

nikic added a comment.

The code basically looks good to me, but there are a number of test failures. At a glance they may be due to changes to variable names in test output.



================
Comment at: llvm/lib/IR/IRBuilder.cpp:345
+    if (!F->returnDoesNotAlias())
+      F->setReturnDoesNotAlias();
+  }
----------------
nikic wrote:
> You can drop the `!F->returnDoesNotAlias()` check (it will be done by setReturnDoesNotAlias).
It looks like you also dropped the call to setReturnDoesNotAlias(), not just the check.


================
Comment at: llvm/lib/IR/IRBuilder.cpp:311
+    ArraySize = ConstantInt::get(IntPtrTy, 1);
+  else if (ArraySize->getType() != IntPtrTy) {
+    ArraySize = CreateIntCast(ArraySize, IntPtrTy, false);
----------------
Omit braces for single-line if.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp:1293
   Constant *size = ConstantInt::get(IntPtrTy, 40);
-  Instruction *SetjmpTable =
-      CallInst::CreateMalloc(SetjmpTableSize, IntPtrTy, IRB.getInt32Ty(), size,
-                             nullptr, nullptr, "setjmpTable");
-  SetjmpTable->setDebugLoc(FirstDL);
-  // CallInst::CreateMalloc may return a bitcast instruction if the result types
-  // mismatch. We need to set the debug loc for the original call too.
-  auto *MallocCall = SetjmpTable->stripPointerCasts();
-  if (auto *MallocCallI = dyn_cast<Instruction>(MallocCall)) {
-    MallocCallI->setDebugLoc(FirstDL);
-  }
   // setjmpTable[0] = 0;
   IRB.SetInsertPoint(SetjmpTableSize);
----------------
I think this comment should be moved above the `IRB.CreateStore(IRB.getInt32(0), SetjmpTable)` line to make sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158861