[PATCH] D98916: [ARM] support symbolic expression as immediate in memory instructions

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers accepted this revision.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.

In D98916#2669915 <https://reviews.llvm.org/D98916#2669915>, @jcai19 wrote:

> Only accept MCExpr in the affected instructions.

Yeah, I think the conservative approach is the way to go here; that way your only functional change is the one you add tests for (ie. `isMemImm12Offset`).  Another thing we could do to be more concise is simply `return false` `if (!isa<MCConstExpr>(Memory.OffsetImm))` for the majority of cases, but `return true` for such a case in `isMemImm12Offset`.  That would avoid the added dyn_casts and resulting indentation.  But in effect it's the same as what you have, so it doesn't matter to me.

I'm ok with this, and thanks for taking the time to work on it.  It would be good to collect another LGTM from another reviewer before submitting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98916