Differential D79778
[CodeGen] Remove buggy handling of input operands in inline asm Authored by vhscampos on May 12 2020, 5:44 AM.
Details
The detection of writes to reserved registers in inline asm cover the This patch removes the buggy part that handles input operands. It seems that this case should be handled in clang instead, as the IR
Diff Detail
Event TimelineComment Actions Ultimately, if we really want to "support" this, I expect the end result should look like the following:
Really, I'm not sure how useful the end result would be, though. This isn't providing any functionality that the user can't already get by just explicitly writing the register in the asm string. And gcc doesn't support equivalent functionality. So ultimately, I'm not sure what you're hoping to accomplish. Is there some existing code which is using this pattern? Comment Actions This effort is to specifically address this bug: https://bugs.llvm.org/show_bug.cgi?id=34165 void fn(int fd, int arg)
{
register int r7 __asm__("r7") = 54;
__asm__ __volatile__ ( "svc 0" : : "r"(r7), "r"(fd), "r"(arg) : "memory");
}gcc does provide such functionality. For example, the code above errors out in gcc. In the case of writing the fixed register in the string itself, gcc and clang are already equivalent.
void foo() {
volatile int a;
__asm__ __volatile("mov r7, %0" : : "r"(a) : "r7");
}
void foo() {
volatile int a;
__asm__ __volatile("mov r7, %0" : : "r"(a) : );
}I look forward to hearing your thoughts on this. Thanks Comment Actions I looked at bug 34165, and concluded that clang and gcc behave the same way, ignoring the different default for -fomit-frame-pointer. Am I missing something? Comment Actions This is the problematic case: void fn(int fd, int arg)
{
register int r7 __asm__("r7");
__asm__ __volatile__ ( "svc 0" : : "r"(r7), "r"(fd), "r"(arg) : "memory");
}Note that, differently from bug 34165, 'r7' is not written to here. gcc gives no warning or error. Clang, however: b.c:4:26: error: write to reserved register 'R7' __asm__ __volatile__ ( "svc 0" : : "r"(r7), "r"(fd), "r"(arg) : "memory"); And this is because the current code considers as an error any use of 'r7' as input operand, regardless of it being written to or not. A correct implementation should trigger an error only when the input operand is written to. This is what motivates me to remove the handling of input operands while we come up with a correct implementation, likely in Clang rather than LLVM as you suggested. The handling of output operands will remain present as it has not been shown to be wrong. Update: I believe then it is a matter of deciding whether or not to remove the current input operand handling while these modifications are worked on. Comment Actions Oh, hmm, somehow I missed that when I looked at this before; gcc actually tracks whether the variable is initialized/assigned-to, and doesn't emit an error if there is no assignment. And, as usual with gcc, whether the error is emitted depends on the optimization level in some cases. That seems completely bizarre, but okay, I guess. If there's some real code depending on that, sure, we can temporarily revert the relevant LLVM changes. Comment Actions That said, nobody has mentioned the uninitialized variable pattern showing up in real code; do you have an example? |