[PATCH] D20406: X86: Don't reset the stack after calls that don't return (PR27117)

Hans Wennborg via llvm-commits llvm-commits at lists.llvm.org
Thu May 19 12:53:08 PDT 2016


hans added inline comments.

================
Comment at: include/llvm/Target/TargetLowering.h:2525
@@ -2524,1 +2524,3 @@
+          Call.doesNotReturn() ||
+          isa<UnreachableInst>(++Call.getInstruction()->getIterator());
       IsVarArg = FTy->isVarArg();
----------------
rnk wrote:
> majnemer wrote:
> > rnk wrote:
> > > Call can be an invoke, in which case it terminates, in which case this may call isa<> on an end iterator, which is bad. I think the easy way to avoid this case is check for !Call.isInvoke() && ....
> > Instead of `isa<UnreachableInst>(++Call.getInstruction()->getIterator())` you could do `isa<UnreachableInst>(Call.getInstruction()->getNextNode())` which is a little shorter (and simpler IMO).
> I think you have to null check getNextNode, though.
Oh right, I forgot invokes are terminators. Adding the !Call.isInvoke() check.

================
Comment at: include/llvm/Target/TargetLowering.h:2525
@@ -2524,1 +2524,3 @@
+          Call.doesNotReturn() ||
+          isa<UnreachableInst>(++Call.getInstruction()->getIterator());
       IsVarArg = FTy->isVarArg();
----------------
hans wrote:
> rnk wrote:
> > majnemer wrote:
> > > rnk wrote:
> > > > Call can be an invoke, in which case it terminates, in which case this may call isa<> on an end iterator, which is bad. I think the easy way to avoid this case is check for !Call.isInvoke() && ....
> > > Instead of `isa<UnreachableInst>(++Call.getInstruction()->getIterator())` you could do `isa<UnreachableInst>(Call.getInstruction()->getNextNode())` which is a little shorter (and simpler IMO).
> > I think you have to null check getNextNode, though.
> Oh right, I forgot invokes are terminators. Adding the !Call.isInvoke() check.
Thanks! I'll do that.

================
Comment at: include/llvm/Target/TargetLowering.h:2525
@@ -2524,1 +2524,3 @@
+          Call.doesNotReturn() ||
+          isa<UnreachableInst>(++Call.getInstruction()->getIterator());
       IsVarArg = FTy->isVarArg();
----------------
hans wrote:
> hans wrote:
> > rnk wrote:
> > > majnemer wrote:
> > > > rnk wrote:
> > > > > Call can be an invoke, in which case it terminates, in which case this may call isa<> on an end iterator, which is bad. I think the easy way to avoid this case is check for !Call.isInvoke() && ....
> > > > Instead of `isa<UnreachableInst>(++Call.getInstruction()->getIterator())` you could do `isa<UnreachableInst>(Call.getInstruction()->getNextNode())` which is a little shorter (and simpler IMO).
> > > I think you have to null check getNextNode, though.
> > Oh right, I forgot invokes are terminators. Adding the !Call.isInvoke() check.
> Thanks! I'll do that.
But not if we know the instruction isn't last in the BB right?


http://reviews.llvm.org/D20406





More information about the llvm-commits mailing list