[PATCH] D83122: Fix crash when getVFABIMappings is called with an indirect call instruction
Francesco Petrogalli via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 13 06:41:58 PDT 2020
fpetrogalli accepted this revision.
fpetrogalli added a comment.
This revision is now accepted and ready to land.
Thank you @sanwou01 ! LGTM, with a nit: please double check those header included are needed before submitting.
Cheers,
Francesco
================
Comment at: llvm/unittests/Analysis/VectorFunctionABITest.cpp:12
#include "llvm/IR/InstIterator.h"
+#include "llvm/IRReader/IRReader.h"
#include "gtest/gtest.h"
----------------
sanwou01 wrote:
> fpetrogalli wrote:
> > Is this needed?
> For the definition of parseAssemblyString, yes.
Are you sure? `parseAssemblyString` is used on line 26, it works even without this include line. Can you double check this before submitting? Same for `<memory>`
================
Comment at: llvm/unittests/Analysis/VectorFunctionABITest.cpp:634
+ LLVMContext C;
+ std::unique_ptr<Module> M = parseIR(C, R"IR(
+define void @call(void () * %f) {
----------------
sanwou01 wrote:
> fpetrogalli wrote:
> > Very elegant, but is this `unique_ptr` needed?
> "borrowed" from other unit tests, so I can only take credit for finding it. I do think the `unique_ptr` is needed: parseAssemblyString passes ownership of the Module to us, so it'd be rude to drop it on the floor.
:facepalm:, yes, I used `unique_ptr` too in the tests class `VFABIParserTest`. My bad.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83122/new/
https://reviews.llvm.org/D83122
More information about the llvm-commits
mailing list