Page MenuHomePhabricator

[CodeGen] Fix calling llvm.var.annotation outside of a basic block.
ClosedPublic

Authored by vsapsai on Feb 12 2019, 2:02 PM.

Details

Summary

When we have an annotated local variable after a function returns, we
generate IR that fails verification with the error

Instruction referencing instruction not embedded in a basic block!

And it means that bitcast referencing alloca doesn't have a parent basic
block.

Fix by checking if we are at an unreachable point and skip emitting
annotations. This approach is similar to the way we emit variable
initializer and debug info.

rdar://problem/46200420

Diff Detail

Repository
rL LLVM

Event Timeline

vsapsai created this revision.Feb 12 2019, 2:02 PM

I've checked and seems like other places where we EmitAnnotationCall should be safe. We do that for globals, function parameters, fields (FieldDecl) and early return isn't applicable in those cases. We can do that for any expressions with __builtin_annotation but that was already working correctly and I've just added a test.

Okay. Is it acceptable for the annotation to simply disappear in this case? I don't really understand the purposes of annotations well enough to judge.

Okay. Is it acceptable for the annotation to simply disappear in this case? I don't really understand the purposes of annotations well enough to judge.

Looks like it is acceptable. The main justification for this behavior is that it is already happening for

return;
int after_return = __builtin_annotation(0, "annotation");

so I think it is correct to have the same IR for

return;
int after_return __attribute__((annotate("annotation"))) = 0;

Also looks like in the code we don't really use this intrinsic. And according to the documentation

This intrinsic allows annotation of local variables with arbitrary
strings. This can be useful for special purpose optimizations that want
to look for these annotations. These have no other defined use; they are
ignored by code generation and optimization.

I think for the special purpose optimizations it would be more convenient not to know about the code after return instead of detecting it and ignoring explicitly.

rjmccall accepted this revision.Feb 26 2019, 10:04 PM

Well, it won't have a guarantee that it won't see unused annotations, but alright, I'm fine with this.

This revision is now accepted and ready to land.Feb 26 2019, 10:04 PM

Thanks for the review.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 28 2019, 6:16 PM