Ahmad Nouralizadeh via llvm-dev
2018-Aug-08 16:29 UTC
[llvm-dev] Error Calling eraseFromParent()
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/c90368aa/attachment.html>
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/164021c0/attachment.html>
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>