[PATCH] D47545: [MC][X86] Allow assembler variable assignment to register name.

[Bill Wendling via Phabricator via llvm-commits]

void added inline comments.


================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:2197
+    SMLoc StartLoc = Parser.getTok().getLoc();
+    // Normal Expression parse fails, check if it could be a register.
+    unsigned RegNo;
----------------
rnk wrote:
> niravd wrote:
> > void wrote:
> > > nickdesaulniers wrote:
> > > > rnk wrote:
> > > > > Does this generalize to more than just registers? Could it include things like `foo = $42`, or `foo = 0x40(%rcx)` as some kind of alias for an accessor? We might want to allow these things so that we don't have to do this fire drill again the next time a Linux developer writes some creative GNU as.
> > > > or tries to do this with another ISA, like arm64.  I guess I'm curious if this is needs to be implemented in the other parsers as well?  We only see the issue on x86 in the kernel currently, but I'd think this kind of functionality would be ISA independent?
> > > Something like `foo = 0x40(%rcx)` should be an illegal expression according to their own documentation....but then again so should using a register as a primary symbol. :-/
> > Extending to generic instruction operands seems relatively straightforward, but given that it's illegal and there's really only a few cases we need to support, I'd prefer to keep this as simple as possible and deal with it as it comes along.
> > 
> > Hopefully, once we have clang consistently building the linux kernel with the integrated assembler (which I believe is now just this) additional things we need to workaround in this we should get quick push back and that'll be enough social pressure to prevent new violations needing compiler support vs. a code rewrite.
> > 
> I'm still curious to know what gas actually supports in practice before we commit to limiting ourselves to things that look like registers. If gas really treats this as a textual macro, that's probably the way we should go.
> 
> They may document that `foo = 0x40(%rcx)` is illegal, but documentation has been known to be wrong.
In an ideal world, we could do that because it would be documented. But as you mentioned their documentation seems faulty. In order to know exactly what gas supports, we would need to unravel its code. That's a fairly complex task, which would take a lot of time and may not be useful.

In this case, it used to be an acceptable practice to allow registers in assembly macros, but it was uncommon. It's possible that its support in gas was unknown to the documentation writers.

IMO, we should go with their documentation and change only when we detect a deviation from it.


Repository:
  rL LLVM

https://reviews.llvm.org/D47545