This is an archive of the discontinued LLVM Phabricator instance.

[IR] Introduce helpers to skip debug instructions (NFC)
ClosedPublic

Authored by vsk on Jun 18 2018, 4:57 PM.

Details

Summary

This patch introduces two helpers to make it easier to ignore debug
intrinsics:

  • Instruction::getNextNonDebugInstruction()

This is just like Instruction::getNextNode(), except that it skips debug
info.

  • skipDebugInfo(BasicBlock::iterator)

A free function which advances a BasicBlock iterator past any debug
info. This is a no-op when the iterator already points to a non-debug
instruction.

Part of: llvm.org/PR37728
Related to: https://reviews.llvm.org/D47874

Diff Detail

Event Timeline

vsk created this revision.Jun 18 2018, 4:57 PM
aprantl added inline comments.Jun 18 2018, 5:32 PM
lib/IR/Instruction.cpp
603

I think it would be more efficient to include these cases in the switch below?

I don't think donothing counts as a meta-instruction, by your definition; yes, it has no effects and no operands, but it isn't ignored by the optimizer (it generally just gets erased, but that's not the same as ignoring it).

llvm.annotation and llvm.ptr.annotation have return values, so they can't be ignored by optimizations, in general. llvm.var.annotation has a pointer operand, so it also can't be ignored by optimizations, in general. This is intentional, so they can be used to experiment with new optimizations.

Given that, a "meta-instruction" is precisely debug info. And I don't think that will change in the future.

vsk added a comment.Jun 19 2018, 10:12 AM

I don't think donothing counts as a meta-instruction, by your definition; yes, it has no effects and no operands, but it isn't ignored by the optimizer (it generally just gets erased, but that's not the same as ignoring it).

Ah, you're right. The langref explains that calls to llvm.donothing may be removed, but aren't generally ignored.

llvm.annotation and llvm.ptr.annotation have return values, so they can't be ignored by optimizations, in general. llvm.var.annotation has a pointer operand, so it also can't be ignored by optimizations, in general. This is intentional, so they can be used to experiment with new optimizations.

Given that, a "meta-instruction" is precisely debug info. And I don't think that will change in the future.

Thanks for providing some background on what llvm.*.annotation is meant to do. My previous understanding of these instructions was that they shouldn't affect whether or not an optimization triggers (the langref states: "they are ignored by code generation and optimization"). I think I was reading too much into this. The language isn't the same as what's used for debug intrinsics ("debug information does not prevent optimizations from happening"). Some optimizations do fail to "kick-in" when annotations are present (jump threading, a few combines), but I suppose that's expected.

Based on this feedback, I think it'd make sense to go with @fhahn's original approach (i.e just introduce a skipDebugInstructions helper).

vsk updated this revision to Diff 151945.Jun 19 2018, 10:52 AM
vsk retitled this revision from [IR] Introduce helpers to skip meta-instructions to [IR] Introduce helpers to skip debug instructions (NFC).
vsk edited the summary of this revision. (Show Details)
efriedma added inline comments.Jun 19 2018, 11:13 AM
lib/IR/Instruction.cpp
602

Maybe this should assert rather than return nullptr? It's probably a logic error if you're calling getNextNonDebugInstruction() on a terminator.

vsk added inline comments.Jun 19 2018, 1:00 PM
lib/IR/Instruction.cpp
602

Hm, but what about loops which advance an iterator using getNextNode()? Here's one from slp-vectorizer (at least, I think 'ScheduleEnd' can be nullptr here): for (auto *I = ScheduleStart; I != ScheduleEnd; I = I->getNextNode()). I'd like getNextNonDebugInst to be a drop-in in these cases.

efriedma accepted this revision.Jun 19 2018, 1:16 PM

LGTM

lib/IR/Instruction.cpp
602

I don't think ScheduleEnd can actually be null there?

But I guess in general, it could be a useful idiom, sure.

This revision is now accepted and ready to land.Jun 19 2018, 1:16 PM
This revision was automatically updated to reflect the committed changes.