[PATCH] D78193: [Sparc] Fixes for the internal assembler

[LemonBoy via Phabricator via llvm-commits]

LemonBoy added a comment.

> It tries to fix a couple of things, so it would probably been better to divide it up into more than one patch and add some more tests, but this is fine. I compared the result with GAS for V8 and it matches.

Sorry about that but splitting the patch into smaller chunks turned up to be harder than writing the patch itself.
I'll send an updated patch soon.



================
Comment at: llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp:289
+  }
+  bool isShiftAmtImm6() const {
+    if (!isImm())
----------------
dcederman wrote:
> Is this for grouping? I think it looks better if there is an empty line between them.
Yep, but I have no strong feelings about that. I'll add an empty line in between.


================
Comment at: llvm/lib/Target/Sparc/MCTargetDesc/SparcMCCodeEmitter.cpp:161
+
+  if (MO.isExpr()) {
+    const MCExpr *Expr = MO.getExpr();
----------------
dcederman wrote:
> Can this be false? In similar code there is usually an assert(MO.isExpr()) instead.
Good catch, I forgot I added the `isImm` check before.
I'll re-check a few things and turn it into an assertion.


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

https://reviews.llvm.org/D78193