fix: incorrect spec about int/float metadata flags #48
Conversation
|
@bettinaheim @idavis could you please review this PR? I wish the repository was configured to suggest default reviewers. Unsure if that's possible. |
| !10 = !{!"i32", !"i64"} | ||
| !11 = !{!"float", !"double"} |
There was a problem hiding this comment.
actually, on trying to implement these changes in a test branch where only i64 are supported, I've found that flags in this format fail linking with the error message:
incorrect number of operands in module flag `!5 = !{!"i64"}`
If there is only one supprted value, should it appear inline rather than as a separate flag?
There was a problem hiding this comment.
Whoops, never mind. I had a different mistake. The flag used as the reference cannot be listed in the !llvm.module.flags. That's why I was getting an error. This syntax is fine.
There was a problem hiding this comment.
Hi @swernli, if I change my copy of the adaptive profile example program to just have !10 = !{!"i64"}, I don't see any error flagged by LLVM. FYI, I also have !4 = !{i32 5, !"int_computations", !10}.
If I try inlining, I still get an error as before this PR.
Perhaps, there is something else going on in your branch? What LLVM version are you using? And what does the attribute for int_computations look like?
There was a problem hiding this comment.
Ah, I see you resolved it.
There was a problem hiding this comment.
Yes, thanks! In fact, I stumbled across a short-hand syntax that the LLVM parser supports where the "sub-flag" can be inlined into the parent. I was able to test this with PyQIR's linking capabilities. So this works:
Closes: #47