Ahmad Nouralizadeh via llvm-dev
2018-Aug-07 23:54 UTC
[llvm-dev] Error Calling eraseFromParent()
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. >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180808/9d592482/attachment.html>
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/ec2d8e6a/attachment.html>
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>