[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/AST/Stmt.cpp:628
+          DiagOffs = CurPtr-StrStart-1;
+          return diag::err_asm_invalid_operand_for_goto_labels;
+        }
----------------
jyu2 wrote:
> rsmith wrote:
> > I'm curious why we're checking this here, when all the other validation for the modifier letter happens in LLVM. Does this check need to be done differently from the others?
> This is to verify following in spec:
> =====
> To reference a label in the assembler template, prefix it with ‘%l’ (lowercase ‘L’) followed by its (zero-based) position in GotoLabels plus the number of input operands. For example, if the asm has three inputs and references two labels, refer to the first label as ‘%l3’ and the second as ‘%l4’).
> 
> Was request from Eli.
Sorry, I think my question was unclear. I understand why you need to do this check somewhere. My question is why this check needs to be *here*, given that all the other checks for the modifier letter are performed elsewhere. Splitting the checks up between multiple locations makes the system harder to maintain.

As it happens, there's a FIXME for this here: http://llvm.org/doxygen/AsmPrinterInlineAsm_8cpp_source.html#l00436 -- and a test case like `void f(void *a) { asm("mov %l0, %%eax" :: "r"(a)); }` currently causes LLVM to assert in a function that's supposed to be validating these modifier letters and producing a clean error if they're bad. So I think that's an LLVM bug, and fixing that LLVM bug would enforce the property we need here (that `%lN` only names labels).


[It's not clear to me whether we should be enforcing that `%lN` only names label //operands//, which would require the check to be in Clang rather than LLVM; GCC doesn't do that, and for instance accepts

```
void f() { asm goto("jmp %l0" :: "i"(&&foo)::foo); foo:; }
```

(at least for x86_64), though this construct doesn't seem hugely useful given that the input operand would need to be a literal address-of-label expression and you'd need to name the target label as a label operand anyway.]


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56571/new/

https://reviews.llvm.org/D56571