Ahmad Nouralizadeh via llvm-dev
2018-Aug-08 17:30 UTC
[llvm-dev] Error Calling eraseFromParent()
LLVM is built in Release mode. The version is 6.0.0. I think that a similar code worked on verison 3.9.0. It is probably a null pointer dereference occurring in eraseFromParent(). I checked and reconfirmed that the instruction had no uses. Perhaps I should rebuild LLVM. Thanks. On Wed, Aug 8, 2018 at 9:03 PM, mayuyu.io <admin at mayuyu.io> wrote:> Hmmmm that’s strange. Do you get an assert when you build LLVM with this > code in Debug Mode or is it simply null pointer dereference? > On a side note, usually when doing transforms similar to yours I use > IRBuilder’s CreateXXX followed by a ReplaceInstWithInst which works > perfectly fine for me. > > Zhang > > 在 2018年8月9日,00:29,Ahmad Nouralizadeh <ahmadnouralizadeh at gmail.com> 写道: > > Hi. Thanks. I changed the code but the problem exists. This is my new code > which is again very simple: > ... > bool runOnFunction(Function &F) override { > > vector<Instruction *> dels; > dels.clear(); > > for (inst_iterator It = inst_begin(&F), Ie = inst_end(&F); It != Ie;) { > Instruction *I = &*(It++); > > if (auto* op = dyn_cast<BinaryOperator>(I)) { > IRBuilder<NoFolder> builder(op); > > Value* lhs = op->getOperand(0); > Value* rhs = op->getOperand(1); > Value* mul = builder.CreateMul(lhs, rhs); > > for (auto& U : op->uses()) { > User* user = U.getUser(); > user->setOperand(U.getOperandNo(), mul); > } > //I->eraseFromParent(); > dels.push_back(I); > } > } > > for (auto &I : dels) { > I->eraseFromParent(); > } > ... > > On Wed, Aug 8, 2018 at 6:16 PM, mayuyu.io <admin at mayuyu.io> wrote: > >> Hi: >> As stated in the documentation you shouldn’t modify it while iterating as >> this invalidates the iterator. Try save the pointers to a vector outside >> your loop and perform erasing in a new loop following your current one. >> >> Zhang >> >> 在 2018年8月8日,07:54,Ahmad Nouralizadeh via llvm-dev < >> llvm-dev at lists.llvm.org> 写道: >> >> Thanks Bjorn! But The problem is still there. >> >> On Wed, Aug 8, 2018 at 2:04 AM, Björn Pettersson A < >> bjorn.a.pettersson at ericsson.com> wrote: >> >>> It looks quite dangerous to erase the instruction I inside the loop when >>> iterating over all instructions in the function. >>> I guess it depends on how the range based iterator is implemented if >>> that works or not. >>> >>> I think that you, for example, can change the range based iteration >>> >>> for (auto &I : instructions(F)) >>> >>> into something like this >>> >>> for (inst_iterator It = inst_begin(&F), Ie = inst_end(&F); It != Ie;) >>> { >>> Instruction *I = &*(It++); >>> ... >>> } >>> >>> That will make sure that you step the iterator before modifying the data >>> structure you iterate over. >>> >>> Regards, >>> Björn >>> ________________________________________ >>> From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Ahmad >>> Nouralizadeh via llvm-dev <llvm-dev at lists.llvm.org> >>> Sent: Tuesday, August 7, 2018 22:26 >>> To: paul.robinson at sony.com >>> Cc: llvm-dev at lists.llvm.org >>> Subject: Re: [llvm-dev] Error Calling eraseFromParent() >>> >>> The code is really simple. But I can not the reason for the segmentation >>> fault. I only know that the eraseFromParent() function leads to it. The >>> whole code is: >>> ... >>> bool runOnFunction(Function &F) override { >>> >>> for (auto &I : instructions(F)) { >>> >>> if (auto* op = dyn_cast<BinaryOperator>(&I)) { >>> IRBuilder<> builder(op); >>> >>> Value* lhs = op->getOperand(0); >>> Value* rhs = op->getOperand(1); >>> Value* mul = builder.CreateMul(lhs, rhs); >>> >>> for (auto& U : op->uses()) { >>> User* user = U.getUser(); >>> user->setOperand(U.getOperandNo(), mul); >>> } >>> >>> I.eraseFromParent(); >>> } >>> >>> } >>> ... >>> And I think that the code worked well with LLVM-3.6.0 that I tested one >>> year ago. Now, I use LLVM-6.0.0. >>> Regards. >>> >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180808/34dee3b8/attachment-0001.html>
Hmmmm. One more guess is that since you used IRBuilder to insert new Instructions, you are still actually modifying the BB and thus invalidated the iterator. I just picked up some old work regarding this and in my example I just wrote something like this: if (BinaryOperator *BO = dyn_cast<BinaryOperator>(Inst)) { BinaryOperator *newBO = llvm::BinaryOperator::Create(newOp, BO->getOperand(0),BO->getOperand(1)); llvm::ReplaceInstWithInst(BO, newBO); } Sorry for the bad formatting, I typed this on mobile. Hope this helps : ) Zhang ------------------ Original ------------------ From: "Ahmad Nouralizadeh"<ahmadnouralizadeh at gmail.com>; Date: Thu, Aug 9, 2018 02:00 AM To: "mayuyu.io"<admin at mayuyu.io>; Cc: "llvm-dev"<llvm-dev at lists.llvm.org>; Subject: Re: [llvm-dev] Error Calling eraseFromParent() LLVM is built in Release mode. The version is 6.0.0. I think that a similar code worked on verison 3.9.0. It is probably a null pointer dereference occurring in eraseFromParent(). I checked and reconfirmed that the instruction had no uses. Perhaps I should rebuild LLVM. Thanks. On Wed, Aug 8, 2018 at 9:03 PM, mayuyu.io <admin at mayuyu.io> wrote: Hmmmm that’s strange. Do you get an assert when you build LLVM with this code in Debug Mode or is it simply null pointer dereference? On a side note, usually when doing transforms similar to yours I use IRBuilder’s CreateXXX followed by a ReplaceInstWithInst which works perfectly fine for me. Zhang 在 2018年8月9日,00:29,Ahmad Nouralizadeh <ahmadnouralizadeh at gmail.com> 写道: Hi. Thanks. I changed the code but the problem exists. This is my new code which is again very simple:... bool runOnFunction(Function &F) override { vector<Instruction *> dels; dels.clear(); for (inst_iterator It = inst_begin(&F), Ie = inst_end(&F); It != Ie;) { Instruction *I = &*(It++); if (auto* op = dyn_cast<BinaryOperator>(I)) { IRBuilder<NoFolder> builder(op); Value* lhs = op->getOperand(0); Value* rhs = op->getOperand(1); Value* mul = builder.CreateMul(lhs, rhs); for (auto& U : op->uses()) { User* user = U.getUser(); user->setOperand(U.getOperandNo(), mul); } //I->eraseFromParent(); dels.push_back(I); } } for (auto &I : dels) { I->eraseFromParent(); } ... On Wed, Aug 8, 2018 at 6:16 PM, mayuyu.io <admin at mayuyu.io> wrote: Hi:As stated in the documentation you shouldn’t modify it while iterating as this invalidates the iterator. Try save the pointers to a vector outside your loop and perform erasing in a new loop following your current one. Zhang 在 2018年8月8日,07:54,Ahmad Nouralizadeh via llvm-dev <llvm-dev at lists.llvm.org> 写道: Thanks Bjorn! But The problem is still there. On Wed, Aug 8, 2018 at 2:04 AM, Björn Pettersson A <bjorn.a.pettersson at ericsson.com> wrote: It looks quite dangerous to erase the instruction I inside the loop when iterating over all instructions in the function. I guess it depends on how the range based iterator is implemented if that works or not. I think that you, for example, can change the range based iteration for (auto &I : instructions(F)) into something like this for (inst_iterator It = inst_begin(&F), Ie = inst_end(&F); It != Ie;) { Instruction *I = &*(It++); ... } That will make sure that you step the iterator before modifying the data structure you iterate over. Regards, Björn ________________________________________ From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Ahmad Nouralizadeh via llvm-dev <llvm-dev at lists.llvm.org> Sent: Tuesday, August 7, 2018 22:26 To: paul.robinson at sony.com Cc: llvm-dev at lists.llvm.org Subject: Re: [llvm-dev] Error Calling eraseFromParent() The code is really simple. But I can not the reason for the segmentation fault. I only know that the eraseFromParent() function leads to it. The whole code is: ... bool runOnFunction(Function &F) override { for (auto &I : instructions(F)) { if (auto* op = dyn_cast<BinaryOperator>(&I)) { IRBuilder<> builder(op); Value* lhs = op->getOperand(0); Value* rhs = op->getOperand(1); Value* mul = builder.CreateMul(lhs, rhs); for (auto& U : op->uses()) { User* user = U.getUser(); user->setOperand(U.getOperandNo(), mul); } I.eraseFromParent(); } } ... And I think that the code worked well with LLVM-3.6.0 that I tested one year ago. Now, I use LLVM-6.0.0. Regards. _______________________________________________ LLVM Developers mailing list llvm-dev at lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180809/44ebdb78/attachment.html>
mats petersson via llvm-dev
2018-Aug-08 18:06 UTC
[llvm-dev] Error Calling eraseFromParent()
On 8 August 2018 at 18:30, Ahmad Nouralizadeh via llvm-dev < llvm-dev at lists.llvm.org> wrote:> LLVM is built in Release mode. The version is 6.0.0. I think that a > similar code worked on verison 3.9.0. It is probably a null pointer > dereference occurring in eraseFromParent(). I checked and reconfirmed that > the instruction had no uses. Perhaps I should rebuild LLVM. Thanks. >Ahmad, I would highly recommend, at least if you are running into problems, that debug using asserts enabled. This will (most of the time) give you a better "explanation" as to what you've done wrong. Not every time, but for many of the "we expect someone to get this wrong at some pont" type of problems. Trying to figure out why something failed in a release build without asserts is often painfully hard, because when you crash, the "reason" for the crash is in code that has been inlined, the actual understanding of what went wrong is several layers of calls above the crash-point. -- Mats> > On Wed, Aug 8, 2018 at 9:03 PM, mayuyu.io <admin at mayuyu.io> wrote: > >> Hmmmm that’s strange. Do you get an assert when you build LLVM with this >> code in Debug Mode or is it simply null pointer dereference? >> On a side note, usually when doing transforms similar to yours I use >> IRBuilder’s CreateXXX followed by a ReplaceInstWithInst which works >> perfectly fine for me. >> >> Zhang >> >> 在 2018年8月9日,00:29,Ahmad Nouralizadeh <ahmadnouralizadeh at gmail.com> 写道: >> >> Hi. Thanks. I changed the code but the problem exists. This is my new >> code which is again very simple: >> ... >> bool runOnFunction(Function &F) override { >> >> vector<Instruction *> dels; >> dels.clear(); >> >> for (inst_iterator It = inst_begin(&F), Ie = inst_end(&F); It != Ie;) { >> Instruction *I = &*(It++); >> >> if (auto* op = dyn_cast<BinaryOperator>(I)) { >> IRBuilder<NoFolder> builder(op); >> >> Value* lhs = op->getOperand(0); >> Value* rhs = op->getOperand(1); >> Value* mul = builder.CreateMul(lhs, rhs); >> >> for (auto& U : op->uses()) { >> User* user = U.getUser(); >> user->setOperand(U.getOperandNo(), mul); >> } >> //I->eraseFromParent(); >> dels.push_back(I); >> } >> } >> >> for (auto &I : dels) { >> I->eraseFromParent(); >> } >> ... >> >> On Wed, Aug 8, 2018 at 6:16 PM, mayuyu.io <admin at mayuyu.io> wrote: >> >>> Hi: >>> As stated in the documentation you shouldn’t modify it while iterating >>> as this invalidates the iterator. Try save the pointers to a vector outside >>> your loop and perform erasing in a new loop following your current one. >>> >>> Zhang >>> >>> 在 2018年8月8日,07:54,Ahmad Nouralizadeh via llvm-dev < >>> llvm-dev at lists.llvm.org> 写道: >>> >>> Thanks Bjorn! But The problem is still there. >>> >>> On Wed, Aug 8, 2018 at 2:04 AM, Björn Pettersson A < >>> bjorn.a.pettersson at ericsson.com> wrote: >>> >>>> It looks quite dangerous to erase the instruction I inside the loop >>>> when iterating over all instructions in the function. >>>> I guess it depends on how the range based iterator is implemented if >>>> that works or not. >>>> >>>> I think that you, for example, can change the range based iteration >>>> >>>> for (auto &I : instructions(F)) >>>> >>>> into something like this >>>> >>>> for (inst_iterator It = inst_begin(&F), Ie = inst_end(&F); It !>>>> Ie;) { >>>> Instruction *I = &*(It++); >>>> ... >>>> } >>>> >>>> That will make sure that you step the iterator before modifying the >>>> data structure you iterate over. >>>> >>>> Regards, >>>> Björn >>>> ________________________________________ >>>> From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Ahmad >>>> Nouralizadeh via llvm-dev <llvm-dev at lists.llvm.org> >>>> Sent: Tuesday, August 7, 2018 22:26 >>>> To: paul.robinson at sony.com >>>> Cc: llvm-dev at lists.llvm.org >>>> Subject: Re: [llvm-dev] Error Calling eraseFromParent() >>>> >>>> The code is really simple. But I can not the reason for the >>>> segmentation fault. I only know that the eraseFromParent() function leads >>>> to it. The whole code is: >>>> ... >>>> bool runOnFunction(Function &F) override { >>>> >>>> for (auto &I : instructions(F)) { >>>> >>>> if (auto* op = dyn_cast<BinaryOperator>(&I)) { >>>> IRBuilder<> builder(op); >>>> >>>> Value* lhs = op->getOperand(0); >>>> Value* rhs = op->getOperand(1); >>>> Value* mul = builder.CreateMul(lhs, rhs); >>>> >>>> for (auto& U : op->uses()) { >>>> User* user = U.getUser(); >>>> user->setOperand(U.getOperandNo(), mul); >>>> } >>>> >>>> I.eraseFromParent(); >>>> } >>>> >>>> } >>>> ... >>>> And I think that the code worked well with LLVM-3.6.0 that I tested one >>>> year ago. Now, I use LLVM-6.0.0. >>>> Regards. >>>> >>> >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >>> >> > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180808/2e2164e9/attachment.html>
Ahmad Nouralizadeh via llvm-dev
2018-Aug-08 19:43 UTC
[llvm-dev] Error Calling eraseFromParent()
Well, the problem was with the LLVM build. I re-extracted the sources and recompiled the whole tool and then SegFaul disappeared! :D Many Thanks all you guys especially Zhang! On Wed, Aug 8, 2018 at 10:36 PM, mats petersson <mats at planetcatfish.com> wrote:> > > On 8 August 2018 at 18:30, Ahmad Nouralizadeh via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> LLVM is built in Release mode. The version is 6.0.0. I think that a >> similar code worked on verison 3.9.0. It is probably a null pointer >> dereference occurring in eraseFromParent(). I checked and reconfirmed that >> the instruction had no uses. Perhaps I should rebuild LLVM. Thanks. >> > Ahmad, > > I would highly recommend, at least if you are running into problems, that > debug using asserts enabled. This will (most of the time) give you a better > "explanation" as to what you've done wrong. Not every time, but for many of > the "we expect someone to get this wrong at some pont" type of problems. > > Trying to figure out why something failed in a release build without > asserts is often painfully hard, because when you crash, the "reason" for > the crash is in code that has been inlined, the actual understanding of > what went wrong is several layers of calls above the crash-point. > > -- > Mats > >> >> On Wed, Aug 8, 2018 at 9:03 PM, mayuyu.io <admin at mayuyu.io> wrote: >> >>> Hmmmm that’s strange. Do you get an assert when you build LLVM with this >>> code in Debug Mode or is it simply null pointer dereference? >>> On a side note, usually when doing transforms similar to yours I use >>> IRBuilder’s CreateXXX followed by a ReplaceInstWithInst which works >>> perfectly fine for me. >>> >>> Zhang >>> >>> 在 2018年8月9日,00:29,Ahmad Nouralizadeh <ahmadnouralizadeh at gmail.com> 写道: >>> >>> Hi. Thanks. I changed the code but the problem exists. This is my new >>> code which is again very simple: >>> ... >>> bool runOnFunction(Function &F) override { >>> >>> vector<Instruction *> dels; >>> dels.clear(); >>> >>> for (inst_iterator It = inst_begin(&F), Ie = inst_end(&F); It != Ie;) { >>> Instruction *I = &*(It++); >>> >>> if (auto* op = dyn_cast<BinaryOperator>(I)) { >>> IRBuilder<NoFolder> builder(op); >>> >>> Value* lhs = op->getOperand(0); >>> Value* rhs = op->getOperand(1); >>> Value* mul = builder.CreateMul(lhs, rhs); >>> >>> for (auto& U : op->uses()) { >>> User* user = U.getUser(); >>> user->setOperand(U.getOperandNo(), mul); >>> } >>> //I->eraseFromParent(); >>> dels.push_back(I); >>> } >>> } >>> >>> for (auto &I : dels) { >>> I->eraseFromParent(); >>> } >>> ... >>> >>> On Wed, Aug 8, 2018 at 6:16 PM, mayuyu.io <admin at mayuyu.io> wrote: >>> >>>> Hi: >>>> As stated in the documentation you shouldn’t modify it while iterating >>>> as this invalidates the iterator. Try save the pointers to a vector outside >>>> your loop and perform erasing in a new loop following your current one. >>>> >>>> Zhang >>>> >>>> 在 2018年8月8日,07:54,Ahmad Nouralizadeh via llvm-dev < >>>> llvm-dev at lists.llvm.org> 写道: >>>> >>>> Thanks Bjorn! But The problem is still there. >>>> >>>> On Wed, Aug 8, 2018 at 2:04 AM, Björn Pettersson A < >>>> bjorn.a.pettersson at ericsson.com> wrote: >>>> >>>>> It looks quite dangerous to erase the instruction I inside the loop >>>>> when iterating over all instructions in the function. >>>>> I guess it depends on how the range based iterator is implemented if >>>>> that works or not. >>>>> >>>>> I think that you, for example, can change the range based iteration >>>>> >>>>> for (auto &I : instructions(F)) >>>>> >>>>> into something like this >>>>> >>>>> for (inst_iterator It = inst_begin(&F), Ie = inst_end(&F); It !>>>>> Ie;) { >>>>> Instruction *I = &*(It++); >>>>> ... >>>>> } >>>>> >>>>> That will make sure that you step the iterator before modifying the >>>>> data structure you iterate over. >>>>> >>>>> Regards, >>>>> Björn >>>>> ________________________________________ >>>>> From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Ahmad >>>>> Nouralizadeh via llvm-dev <llvm-dev at lists.llvm.org> >>>>> Sent: Tuesday, August 7, 2018 22:26 >>>>> To: paul.robinson at sony.com >>>>> Cc: llvm-dev at lists.llvm.org >>>>> Subject: Re: [llvm-dev] Error Calling eraseFromParent() >>>>> >>>>> The code is really simple. But I can not the reason for the >>>>> segmentation fault. I only know that the eraseFromParent() function leads >>>>> to it. The whole code is: >>>>> ... >>>>> bool runOnFunction(Function &F) override { >>>>> >>>>> for (auto &I : instructions(F)) { >>>>> >>>>> if (auto* op = dyn_cast<BinaryOperator>(&I)) { >>>>> IRBuilder<> builder(op); >>>>> >>>>> Value* lhs = op->getOperand(0); >>>>> Value* rhs = op->getOperand(1); >>>>> Value* mul = builder.CreateMul(lhs, rhs); >>>>> >>>>> for (auto& U : op->uses()) { >>>>> User* user = U.getUser(); >>>>> user->setOperand(U.getOperandNo(), mul); >>>>> } >>>>> >>>>> I.eraseFromParent(); >>>>> } >>>>> >>>>> } >>>>> ... >>>>> And I think that the code worked well with LLVM-3.6.0 that I tested >>>>> one year ago. Now, I use LLVM-6.0.0. >>>>> Regards. >>>>> >>>> >>>> _______________________________________________ >>>> LLVM Developers mailing list >>>> llvm-dev at lists.llvm.org >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>> >>>> >>> >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180809/1da10cf6/attachment.html>