Allow indirect operands to be used as inputs for inline asm #29543
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pnkfelix (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
Thanks! Could you add some test cases as well? |
|
I've added tests for "*m" and "=*m". It seems that "+*m" is broken in LLVM, so I didn't add it. Actually it seems that LLVM doesn't even support "+" for specifying read/write operands (it's not mentioned in the documentation). Clang translates it into an input/output pair bound to the same register. Maybe we should remove support for "+" operands since they can be trivially emulated using an input/output operand pair? This would be a breaking change though... |
|
@Amanieu why not implement that same trivial emulation of |
|
I'm afraid I'm not familiar enough with the compiler to do that. Note that we need to make sure the operand expression isn't evaluated twice. Also I think keeping asm! as close to LLVM's inline asm is preferable for now, so that a higher-level wrapper can be designed on top of it later. I had a look at the LLVM code, and it seems that it's not just "+m" that isn't supported, "+" isn't supported in the constraint string at all. This means that any code currently using "+" constraints is broken and is simply working by luck (LLVM seems to just ignore invalid constraints and pick an arbitrary register). |
|
The main intent of this PR is to allow people currently using inline asm to have a way to specify memory operands that isn't broken, which plain "m" and "=m" is. |
|
⌛ Testing commit 59c5191 with merge effcd29... |
The "m" memory constraint in inline assembly is broken (generates incorrect code or triggers LLVM asserts) and should not be used. Instead, indirect memory operands should be used with "\*m", "=\*m" and "+\*m". Clang does this transparently by transforming "m" constraints into "\*m" indirect constraints, but for now just being able to use "\*m" directly is enough since asm! isn't stable. While "\*m" works fine as an input operand, "=\*m" and "+\*m" need to be specified as input operands because they take a pointer value as an input. This PR relaxes the constraint checker to allow constraints starting with "=" or "+" if the constraint string contains a "\*", which indicates an indirect operand. This (indirectly) fixes these issues: #29382, #16383 and #13366. The code will need to be changed to use "\*m" instead of "m".
This PR reverts #29543 and instead implements proper support for "=*m" and "+*m" indirect output operands. This provides a framework on top of which support for plain memory operands ("m", "=m" and "+m") can be implemented. This also fixes the liveness analysis pass not handling read/write operands correctly.
The "m" memory constraint in inline assembly is broken (generates incorrect code or triggers LLVM asserts) and should not be used. Instead, indirect memory operands should be used with "*m", "=*m" and "+*m".
Clang does this transparently by transforming "m" constraints into "*m" indirect constraints, but for now just being able to use "*m" directly is enough since asm! isn't stable.
While "*m" works fine as an input operand, "=*m" and "+m" need to be specified as input operands because they take a pointer value as an input. This PR relaxes the constraint checker to allow constraints starting with "=" or "+" if the constraint string contains a "", which indicates an indirect operand.
This (indirectly) fixes these issues: #29382, #16383 and #13366. The code will need to be changed to use "*m" instead of "m".