This is an archive of the discontinued LLVM Phabricator instance.

[LangRef] Require elementtype attribute for indirect inline asm operands
ClosedPublic

Authored by nikic on Jan 3 2022, 3:49 AM.

Details

Summary

Indirect inline asm operands may require the materialization of a memory access according to the pointer element type. As this will no longer be available with opaque pointers, we require it to be explicitly annotated using the elementtype attribute, for example:

define void @test(i32* %p, i32 %x) {
  call void asm "addl $1, $0", "=*rm,r"(i32* elementtype(i32) %p, i32 %x)
  ret void
}

This patch only includes the LangRef change and Verifier updates to allow adding the elementtype attribute in this position. It does not yet enforce this, as this will require changes on the clang side (and test updates) first.

Something I'm a bit unsure about is whether we really need the elementtype for all indirect constraints, rather than only indirect register constraints. I think indirect memory constraints might not need it (though the backend code is written in a way that does require it). I think it's okay to just make this a general requirement though, as this means we don't need to carefully deal with multiple or alternative constraints. In addition, I believe that MemorySanitizer benefits from having the element type even in cases where it may not be strictly necessary for normal lowering (https://github.com/llvm/llvm-project/blob/cd2b050fa4995b75b9c36fae16c0d9f105b67585/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp#L4066).

Diff Detail

Unit TestsFailed

Event Timeline

nikic created this revision.Jan 3 2022, 3:49 AM
nikic requested review of this revision.Jan 3 2022, 3:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2022, 3:49 AM
lebedev.ri accepted this revision.Jan 3 2022, 4:12 AM

Seems fine to me.
Thanks.

llvm/test/Verifier/inline-asm-indirect-operand.ll
15–20

Worth testing that the checking is done for call/invoke/asm goto, not just call?

This revision is now accepted and ready to land.Jan 3 2022, 4:12 AM
This revision was landed with ongoing or failed builds.Jan 4 2022, 1:02 AM
This revision was automatically updated to reflect the committed changes.
nikic marked an inline comment as done.
jrtc27 added a subscriber: jrtc27.Jan 5 2022, 7:55 AM

Will there be an auto upgrade for this if a non-opaque pointer is used?

nikic added a comment.Jan 5 2022, 7:59 AM

Will there be an auto upgrade for this if a non-opaque pointer is used?

Yes, old bitcode will be auto-upgraded.