Sanjoy Das via llvm-dev
2016-Aug-24 07:12 UTC
[llvm-dev] LLVM 3.9 RC2's SCCP pass removing calls to external functions?!
Hi Félix, Sanjoy Das wrote: > Félix Cloutier via llvm-dev wrote: > > Assuming that this is a bug, what are the next steps? > > Looks like you already have a very small test case -- have you tried > sticking it in a debugger to see why SCCP thinks removing the call is > okay? > > Alternatively, file a bug at llvm.org/bugs and someone will get to it. The third thing you can do is run a bisection to find the problematic commit, since you said this was a regression. -- Sanjoy
Sanjoy Das via llvm-dev
2016-Aug-24 07:38 UTC
[llvm-dev] LLVM 3.9 RC2's SCCP pass removing calls to external functions?!
Btw, looks like the bug is in runSCCP in
for (BasicBlock::iterator BI = BB.begin(), E = BB.end(); BI != E;) {
Instruction *Inst = &*BI++;
if (Inst->getType()->isVoidTy() || isa<TerminatorInst>(Inst))
continue;
if (tryToReplaceInstWithConstant(Solver, Inst,
true /* shouldEraseFromParent */)) {
// Hey, we just changed something!
MadeChanges = true;
++NumInstRemoved;
}
It should not be passing in true for shouldEraseFromParent. I think
the right fix here is to not have the shouldEraseFromParent parameter
at all, but in tryToReplaceInstWithConstant to do:
// replace Inst with constant
llvm::RecursivelyDeleteTriviallyDeadInstructions(Inst, TLI);
Let me know if you want to take a crack at this, otherwise I'll get to
it this week.
-- Sanjoy
On Wed, Aug 24, 2016 at 12:12 AM, Sanjoy Das
<sanjoy at playingwithpointers.com> wrote:> Hi Félix,
>
> Sanjoy Das wrote:
>> Félix Cloutier via llvm-dev wrote:
>> > Assuming that this is a bug, what are the next steps?
>>
>> Looks like you already have a very small test case -- have you tried
>> sticking it in a debugger to see why SCCP thinks removing the call is
>> okay?
>>
>> Alternatively, file a bug at llvm.org/bugs and someone will get to it.
>
> The third thing you can do is run a bisection to find the problematic
> commit, since you said this was a regression.
>
> -- Sanjoy
--
Sanjoy Das
http://playingwithpointers.com
Alexandre Isoard via llvm-dev
2016-Aug-24 07:48 UTC
[llvm-dev] LLVM 3.9 RC2's SCCP pass removing calls to external functions?!
I am probably stating the obvious, but if the function is side-effect free (onlyReadsMemory) it is valid to remove it. But I am guessing that does not belong to this pass. On Wed, Aug 24, 2016 at 8:38 AM, Sanjoy Das via llvm-dev < llvm-dev at lists.llvm.org> wrote:> Btw, looks like the bug is in runSCCP in > > for (BasicBlock::iterator BI = BB.begin(), E = BB.end(); BI != E;) { > Instruction *Inst = &*BI++; > if (Inst->getType()->isVoidTy() || isa<TerminatorInst>(Inst)) > continue; > > if (tryToReplaceInstWithConstant(Solver, Inst, > true /* shouldEraseFromParent */)) { > // Hey, we just changed something! > MadeChanges = true; > ++NumInstRemoved; > } > > It should not be passing in true for shouldEraseFromParent. I think > the right fix here is to not have the shouldEraseFromParent parameter > at all, but in tryToReplaceInstWithConstant to do: > > // replace Inst with constant > llvm::RecursivelyDeleteTriviallyDeadInstructions(Inst, TLI); > > Let me know if you want to take a crack at this, otherwise I'll get to > it this week. > > -- Sanjoy > > > On Wed, Aug 24, 2016 at 12:12 AM, Sanjoy Das > <sanjoy at playingwithpointers.com> wrote: > > Hi Félix, > > > > Sanjoy Das wrote: > >> Félix Cloutier via llvm-dev wrote: > >> > Assuming that this is a bug, what are the next steps? > >> > >> Looks like you already have a very small test case -- have you tried > >> sticking it in a debugger to see why SCCP thinks removing the call is > >> okay? > >> > >> Alternatively, file a bug at llvm.org/bugs and someone will get to it. > > > > The third thing you can do is run a bisection to find the problematic > > commit, since you said this was a regression. > > > > -- Sanjoy > > > > -- > Sanjoy Das > http://playingwithpointers.com > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-- *Alexandre Isoard* -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160824/3458d415/attachment.html>